-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Compute 4 Byte selector from SolError and SolFunction #24
Conversation
Just saw this, reviewing. |
crates/ast/src/util.rs
Outdated
func: SolFunction, | ||
err: SolError, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why does this function take both a func
and err
? It should be either or
crates/ast/src/util.rs
Outdated
DynSolType::Address => "address", | ||
DynSolType::Uint(_) => "uint256", | ||
DynSolType::String => "string", | ||
_ => panic!("Unsupported type: {:?}", arg.0), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You don't support all types. I think Alloy might have a built-in way, try .to_string()
crates/ast/src/util.rs
Outdated
@@ -53,3 +56,34 @@ pub fn u256_as_push(value: U256) -> Opcode { | |||
_ => unreachable!(), | |||
} | |||
} | |||
|
|||
type Selector = (FixedBytes<4>, FixedBytes<4>); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove unused
Hey @willco-1, thanks for the contribution. Looking great. Before merging in could you do a few more small things:
thanks! |
Nice, almost there. Could you get the tests to pass now? |
Clippy 🙃 |
Also fixed CI so that you should be able to run it yourself, apologize for the small back and forth 😅 |
Closes #6 |
Merged in, decided to fix the nit myself. Thanks for the contribution! |
This aims to solve #6. I think that there might be a better way to do this, perhaps. Also, I am sure there will be stylistic things to change here.. so I am opening it as a Draft Pr. Please advise on any notes.