Fix function selector & argument encoding #39

0xOmarA · 2025-07-13T16:43:11Z

Summary

This PR fixes some issues that we had in computing the function selector and in the encoding of arguments for functions

Description

Tl;Dr: I made the following changes in this PR:

Corrected the way in which the function selector was computed.

Changed the logic that parses the Input.calldata -> Bytes to align more with matterlabs, which fixed a bug I encountered.

I encountered an issue where the function selector that we computed didn't seem to be correct as can be seen from the following logs:

  2025-07-13T13:37:06.060606Z ERROR revive_dt_core::driver: Failed to construct legacy transaction: Function with selector [223, 254, 173, 208] not found in ABI for the instance "Main"
    at crates/core/src/driver/mod.rs:145 on main ThreadId(1)
    in revive_dt_core::driver::Executing case with case: "first", case_idx: 0
    in retester::Running driver with metadata_file_path: "era-compiler-tests/solidity/complex/array_one_element/test.json"

  2025-07-13T13:37:06.060626Z ERROR revive_dt_core::driver: Leader execution failed for Main: Function with selector [223, 254, 173, 208] not found in ABI for the instance "Main"
    at crates/core/src/driver/mod.rs:476 on main ThreadId(1)
    in revive_dt_core::driver::Executing case with case: "first", case_idx: 0
    in retester::Running driver with metadata_file_path: "era-compiler-tests/solidity/complex/array_one_element/test.json"

After doing some digging, I discovered that we were attempting to compute the function selector from the function name alone and without making use of the function arguments types.

I corrected that to align more with the matter-labs-tester: now we obtain the function selector from the ABI by function name. It looks like the matter-labs team do not account for function overloads, and therefore we didn't account for them either. It seems like they enforce that there will never be function overloads in tests (at least in the functions that gets called from the tests).

I then faced the following error after correcting the way that we compute the function selector:

  2025-07-13T14:38:54.180925Z ERROR revive_dt_core::driver: Failed to construct legacy transaction: Unsupported type: uint256[1]
    at crates/core/src/driver/mod.rs:145 on main ThreadId(1)
    in revive_dt_core::driver::Executing case with case: "first", case_idx: 0
    in retester::Running driver with metadata_file_path: "era-compiler-tests/solidity/complex/array_one_element/test.json"

  2025-07-13T14:38:54.180949Z ERROR revive_dt_core::driver: Leader execution failed for Main: Unsupported type: uint256[1]
    at crates/core/src/driver/mod.rs:476 on main ThreadId(1)
    in revive_dt_core::driver::Executing case with case: "first", case_idx: 0
    in retester::Running driver with metadata_file_path: "era-compiler-tests/solidity/complex/array_one_element/test.json"

Digging deeper, it looked like our implementation for argument encoding different from the implementation of matter-labs where we were trying to support more types than they support. Additionally, we attempt to decode the arguments based on the ABI of the method whereas they decode the arguments without the ABI context at all. In this case, I think that it's also best that we align with their implementation since they author the test suite and therefore it's better that our parser aligns with theirs. Therefore, I changed the implementation of Input -> Calldata to not rely on the ABI and to have the same parsing logic as them.

These two changes have fixed the issues that I've been seeing, and I think that I finally got my first test case to work 🎉!

0xOmarA · 2025-07-13T16:53:09Z

crates/format/src/input.rs

    }

-    #[test]
-    fn test_encoded_input_bool() {


Removed this since the matterlabs parser doesn't support parsing of booleans and we've aligned our parsing implementation with theirs.

0xOmarA · 2025-07-13T16:53:19Z

crates/format/src/input.rs

-    }
-
-    #[test]
-    fn test_encoded_input_string() {


Removed this since the matterlabs parser doesn't support parsing of strings and we've aligned our parsing implementation with theirs.

0xOmarA · 2025-07-13T16:53:31Z

crates/format/src/input.rs

-    }
-
-    #[test]
-    fn test_encoded_input_uint256_array() {


Removed this since the matterlabs parser doesn't support parsing of arbitrary length arrays and we've aligned our parsing implementation with theirs.

0xOmarA · 2025-07-13T20:00:44Z

crates/format/src/input.rs

+        // add logic that does their resolution in the future, perhaps through some kind of system
+        // context API that we pass down to the resolution function that allows it to make calls to
+        // the node to perform these resolutions.
+        let is_unsupported = [


Highlighting this bit for reviewers. We don't currently support these variables in our testing framework. It's certainly possible to support them. I think that the best way would be for us to add some kind of ResolverApi in the future that is passed to the resolve function that allows for it to fetch such information.

I'd assume they can be fetched via the ethereum RPC?

Yup, I think that we should be able to get all of this information from the RPC. I was hoping to first implement #40 to make async communication with the RPC slightly simpler before adding support for them. What do you think?

@xermicus I have added support for these in #43

xermicus · 2025-07-14T09:19:07Z

Cargo.toml

    "json",
    "env-filter",
 ] }
+web3 = { version = "0.19.0", default-features = false }


Is there anything missing in alloy which the web3 crate provides? I don't think so and would like to stick with alloy only (also this was last released 2 years ago).

Good point, I think that we can do the same with alloy::primitives::U256. Initially, I wanted to not modify the matterlabs implementation as to have an easier way to verify its correctness, but you're right a lot of these methods have counterparts in alloy::primitives::U256.

I have made this change so we're no longer using the web3 crate.

Ah I see. Very nice thanks. Helps maintainability greatly to have only a single crate for helpers like this.

xermicus · 2025-07-14T09:22:57Z

crates/format/src/input.rs

+        // add logic that does their resolution in the future, perhaps through some kind of system
+        // context API that we pass down to the resolution function that allows it to make calls to
+        // the node to perform these resolutions.
+        let is_unsupported = [


I'd assume they can be fetched via the ethereum RPC?

Fix function selector and argument encoding

e3723e7

0xOmarA force-pushed the bugfix/argument-encoding branch from 596a572 to e3723e7 Compare July 13, 2025 16:52

0xOmarA commented Jul 13, 2025

View reviewed changes

0xOmarA mentioned this pull request Jul 13, 2025

Proposal to modify the async runtime #40

Closed

0xOmarA commented Jul 13, 2025

View reviewed changes

Avoid extra buffer allocation

2373872

xermicus reviewed Jul 14, 2025

View reviewed changes

0xOmarA added 2 commits July 14, 2025 18:27

Remove reliance on the web3 crate

43e0d0e

Merge remote-tracking branch 'origin/main' into bugfix/argument-encoding

075c823

0xOmarA mentioned this pull request Jul 14, 2025

Update the async runtime #42

Merged

Fix tests

83c20b1

0xOmarA mentioned this pull request Jul 14, 2025

Support other matterlabs variables #43

Merged

3 tasks

xermicus approved these changes Jul 15, 2025

View reviewed changes

0xOmarA added this pull request to the merge queue Jul 15, 2025

Merged via the queue into main with commit c2e65f9 Jul 15, 2025
5 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix function selector & argument encoding #39

Fix function selector & argument encoding #39

Uh oh!

0xOmarA commented Jul 13, 2025 •

edited

Loading

Uh oh!

0xOmarA Jul 13, 2025

Uh oh!

0xOmarA Jul 13, 2025

Uh oh!

0xOmarA Jul 13, 2025

Uh oh!

0xOmarA Jul 13, 2025 •

edited

Loading

Uh oh!

xermicus Jul 14, 2025

Uh oh!

0xOmarA Jul 14, 2025

Uh oh!

0xOmarA Jul 15, 2025

Uh oh!

xermicus Jul 14, 2025

Uh oh!

0xOmarA Jul 14, 2025

Uh oh!

xermicus Jul 15, 2025

Uh oh!

xermicus Jul 14, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix function selector & argument encoding #39

Fix function selector & argument encoding #39

Uh oh!

Conversation

0xOmarA commented Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

0xOmarA Jul 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

0xOmarA commented Jul 13, 2025 •

edited

Loading

0xOmarA Jul 13, 2025 •

edited

Loading