-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Fix calling contracts with structs #80
Conversation
e7da468
to
64487ed
Compare
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.
lgtm
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.
Great stuff! 🔥
Just one clarification comment, but feel free to merge whenever.
@@ -258,19 +310,31 @@ export default class Provider { | |||
request: TransactionRequest; | |||
wait: () => Promise<TransactionResult & { data: Uint8Array }>; | |||
}> { | |||
const dataArray = arrayify(data); | |||
const functionSelector = dataArray.slice(0, 8); | |||
const isStructArg = dataArray.slice(8, 16).some((b) => b === 0x01); |
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.
I'm just curious and a bit confused: how come we're confirming it's a struct arg by checking b === 0x01
?
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.
It's an ABI-encoded boolean and we are checking if it has any bytes matching 0x01
in it to see if we should interpret it as true or false.
I was going to make these lines prettier, but I decided to leave a comment instead (and forgot the comment).
I have identified multiple ways to implement this and each one was uglier than the other. Then I realized we only support exactly one argument and when we add support for 0 and 1+ I will have to go over this again. So I just left it like this and will get back to it when it's clear how non-1 args are going to work.
No description provided.