Merge branch 'main' into elmattic/optional-eth-mappings

CHANGELOG.md

@@ -77,6 +77,8 @@
 
 - [#5342](https://github.com/ChainSafe/forest/pull/5342) Improved the ergonomics of handling addresses in the `Filecoin.EthGetLogs` and `Filecoin.EthNewFilter` to accept both single addresses and arrays of addresses. This conforms to the Ethereum RPC API.
 
+- [#5346](https://github.com/ChainSafe/forest/pull/5346) `Filecoin.EthGetBlockReceipts` and `Filecoin.EthGetBlockReceiptsLimited` now accepts predefined block parameters on top of the block hash, e.g., `latest`, `earliest`, `pending`.
+
 ### Changed
 
 - [#5237](https://github.com/ChainSafe/forest/pull/5237) Stylistic changes to FIL pretty printing.
@@ -109,6 +111,8 @@
 
 - [#5213](https://github.com/ChainSafe/forest/issues/5213) Fix incorrect results for the `Filecoin.EthGetLogs` RPC method on ranges that include null tipsets.
 
+- [#5345](https://github.com/ChainSafe/forest/pull/5345) Fixed handling of odd-length hex strings in some Eth RPC methods. Now, the methods should not return error if provided with, e.g., `0x0` (which would be expanded to `0x00`).
+
 ## Forest v.0.23.3 "Plumber"
 
 Mandatory release for calibnet node operators. It fixes a sync error at epoch 2281645.

src/lotus_json/mod.rs

@@ -347,34 +347,39 @@ pub mod hexify_bytes {
 
 pub mod hexify_vec_bytes {
     use super::*;
-    use std::fmt::Write;
+    use std::borrow::Cow;
 
     pub fn serialize<S>(value: &[u8], serializer: S) -> Result<S::Ok, S::Error>
     where
         S: Serializer,
     {
-        let mut s = String::with_capacity(2 + value.len() * 2);
-        s.push_str("0x");
-        for b in value {
-            write!(s, "{:02x}", b).expect("failed to write to string");
-        }
-        serializer.serialize_str(&s)
+        let mut s = vec![0; 2 + value.len() * 2];
+        s.get_mut(0..2)
+            .expect("len is correct")
+            .copy_from_slice(b"0x");
+        hex::encode_to_slice(value, s.get_mut(2..).expect("len is correct"))
+            .map_err(serde::ser::Error::custom)?;
+        serializer.serialize_str(std::str::from_utf8(&s).expect("valid utf8"))
     }
 
     pub fn deserialize<'de, D>(deserializer: D) -> Result<Vec<u8>, D::Error>
     where
         D: Deserializer<'de>,
     {
         let s = String::deserialize(deserializer)?;
-        if (s.len() >= 2 && s.len() % 2 == 0) && s.get(..2).expect("failed to get prefix") == "0x" {
-            let result: Result<Vec<u8>, _> = (2..s.len())
-                .step_by(2)
-                .map(|i| u8::from_str_radix(s.get(i..i + 2).expect("failed to get slice"), 16))
-                .collect();
-            result.map_err(serde::de::Error::custom)
+        let s = Cow::from(s.strip_prefix("0x").unwrap_or(&s));
+
+        // Pad with 0 if odd length. This is necessary because [`hex::decode`] requires an even
+        // number of characters, whereas a valid input is also `0x0`.
+        let s = if s.len() % 2 == 0 {
+            s
         } else {
-            Err(serde::de::Error::custom("Invalid hex"))
-        }
+            let mut s = s.into_owned();
+            s.insert(0, '0');
+            Cow::Owned(s)
+        };
+
+        hex::decode(s.as_ref()).map_err(serde::de::Error::custom)
     }
 }
 
@@ -601,3 +606,53 @@ mod fixme {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use ipld_core::serde::SerdeError;
+    use serde::de::{value::StringDeserializer, IntoDeserializer};
+
+    #[derive(Debug, Deserialize, Serialize, PartialEq)]
+    struct HexifyVecBytesTest {
+        #[serde(with = "hexify_vec_bytes")]
+        value: Vec<u8>,
+    }
+
+    #[test]
+    fn test_hexify_vec_bytes_serialize() {
+        let cases = [(vec![], "0x"), (vec![0], "0x00"), (vec![42, 66], "0x2a42")];
+
+        for (input, expected) in cases.into_iter() {
+            let hexify = HexifyVecBytesTest { value: input };
+            let serialized = serde_json::to_string(&hexify).unwrap();
+            self::assert_eq!(serialized, format!("{{\"value\":\"{}\"}}", expected));
+        }
+    }
+
+    #[test]
+    fn test_hexify_vec_bytes_deserialize() {
+        let cases = [
+            ("0x", vec![]),
+            ("0x0", vec![0]),
+            ("0xF", vec![15]),
+            ("0x2a42", vec![42, 66]),
+            ("0x2A42", vec![42, 66]),
+        ];
+
+        for (input, expected) in cases.into_iter() {
+            let deserializer: StringDeserializer<SerdeError> =
+                String::from_str(input).unwrap().into_deserializer();
+            let deserialized = hexify_vec_bytes::deserialize(deserializer).unwrap();
+            self::assert_eq!(deserialized, expected);
+        }
+
+        let fail_cases = ["cthulhu", "x", "0xazathoth"];
+        for input in fail_cases.into_iter() {
+            let deserializer: StringDeserializer<SerdeError> =
+                String::from_str(input).unwrap().into_deserializer();
+            let deserialized = hexify_vec_bytes::deserialize(deserializer);
+            assert!(deserialized.is_err());
+        }
+    }
+}

src/rpc/methods/eth.rs

@@ -1409,11 +1409,11 @@ impl RpcMethod<2> for EthGetBlockByNumber {
 
 async fn get_block_receipts<DB: Blockstore + Send + Sync + 'static>(
     ctx: &Ctx<DB>,
-    block_hash: EthHash,
+    block_param: BlockNumberOrHash,
     // TODO(forest): https://github.com/ChainSafe/forest/issues/5177
     _limit: Option<usize>,
 ) -> Result<Vec<EthTxReceipt>, ServerError> {
-    let ts = get_tipset_from_hash(ctx.chain_store(), &block_hash)?;
+    let ts = tipset_by_block_number_or_hash(ctx.chain_store(), block_param)?;
     let ts_ref = Arc::new(ts);
     let ts_key = ts_ref.key();
 
@@ -1444,35 +1444,35 @@ pub enum EthGetBlockReceipts {}
 impl RpcMethod<1> for EthGetBlockReceipts {
     const NAME: &'static str = "Filecoin.EthGetBlockReceipts";
     const NAME_ALIAS: Option<&'static str> = Some("eth_getBlockReceipts");
-    const PARAM_NAMES: [&'static str; 1] = ["block_hash"];
+    const PARAM_NAMES: [&'static str; 1] = ["block_param"];
     const API_PATHS: ApiPaths = ApiPaths::V1;
     const PERMISSION: Permission = Permission::Read;
-    type Params = (EthHash,);
+    type Params = (BlockNumberOrHash,);
     type Ok = Vec<EthTxReceipt>;
 
     async fn handle(
         ctx: Ctx<impl Blockstore + Send + Sync + 'static>,
-        (block_hash,): Self::Params,
+        (block_param,): Self::Params,
     ) -> Result<Self::Ok, ServerError> {
-        get_block_receipts(&ctx, block_hash, None).await
+        get_block_receipts(&ctx, block_param, None).await
     }
 }
 
 pub enum EthGetBlockReceiptsLimited {}
 impl RpcMethod<2> for EthGetBlockReceiptsLimited {
     const NAME: &'static str = "Filecoin.EthGetBlockReceiptsLimited";
     const NAME_ALIAS: Option<&'static str> = Some("eth_getBlockReceiptsLimited");
-    const PARAM_NAMES: [&'static str; 2] = ["block_hash", "limit"];
+    const PARAM_NAMES: [&'static str; 2] = ["block_param", "limit"];
     const API_PATHS: ApiPaths = ApiPaths::V1;
     const PERMISSION: Permission = Permission::Read;
-    type Params = (EthHash, ChainEpoch);
+    type Params = (BlockNumberOrHash, ChainEpoch);
     type Ok = Vec<EthTxReceipt>;
 
     async fn handle(
         ctx: Ctx<impl Blockstore + Send + Sync + 'static>,
-        (block_hash, limit): Self::Params,
+        (block_param, limit): Self::Params,
     ) -> Result<Self::Ok, ServerError> {
-        get_block_receipts(&ctx, block_hash, Some(limit as usize)).await
+        get_block_receipts(&ctx, block_param, Some(limit as usize)).await
     }
 }
 

src/tool/subcommands/api_cmd/api_compare_tests.rs

@@ -1388,11 +1388,30 @@ fn eth_tests_with_tipset<DB: Blockstore>(store: &Arc<DB>, shared_tipset: &Tipset
             ))
             .unwrap(),
         ),
-        RpcTest::identity(EthGetBlockReceipts::request((block_hash.clone(),)).unwrap()),
+        RpcTest::identity(
+            EthGetBlockReceipts::request((BlockNumberOrHash::from_block_hash_object(
+                block_hash.clone(),
+                true,
+            ),))
+            .unwrap(),
+        ),
+        // Nodes might be synced to different epochs, so we can't assert the exact result here.
+        // Regardless, we want to check if the node returns a valid response and accepts predefined
+        // values.
+        RpcTest::basic(
+            EthGetBlockReceipts::request((BlockNumberOrHash::from_predefined(Predefined::Latest),))
+                .unwrap(),
+        ),
         RpcTest::identity(
             EthGetBlockTransactionCountByHash::request((block_hash.clone(),)).unwrap(),
         ),
-        RpcTest::identity(EthGetBlockReceiptsLimited::request((block_hash.clone(), 800)).unwrap()),
+        RpcTest::identity(
+            EthGetBlockReceiptsLimited::request((
+                BlockNumberOrHash::from_block_hash_object(block_hash.clone(), true),
+                800,
+            ))
+            .unwrap(),
+        ),
         RpcTest::identity(
             EthGetBlockTransactionCountByNumber::request((EthInt64(shared_tipset.epoch()),))
                 .unwrap(),