-
Notifications
You must be signed in to change notification settings - Fork 20.4k
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
Signer pt 1 #15702
Signer pt 1 #15702
Conversation
// into a go type with accordance with the ABI spec. | ||
func toGoType(index int, t Type, output []byte) (interface{}, error) { | ||
func ToGoType(index int, t Type, output []byte) (interface{}, error) { |
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 not happy about this change because you should be able to use
var v interface{}
abi.Unpack(&v, "method", calldata)
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'd be glad for some suggestions on how to do it 'properly' -- I did it the only way I saw possible. See https://github.com/holiman/go-ethereum/blob/signer_mhs/cmd/signer/abihelper.go#L65 for more context.
I didn't know you could decode into an interface{}
, I thought you had to allocate a suitable struct beforehand. I'll try your suggestion.
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.
That doesn't quite work, the Unpack
function aims at decoding returndata, not inputs.
It panics
at e.g.
marshalledValue, err := ToGoType(0, method.Outputs[0].Type, output)
Because I'm throwing it input, and have not even defined any output. A hacky way to go about it would be to retranslate inputs to outputs in my abi, so that instead of supplying
{"type":"function","name":"send","inputs":[{"type":"uint256"}]}
I would supply
{"type":"function","name":"send","outputs":[{"type":"uint256"}]}
But is that nice?
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 possible to make Unpack
a bit more generic and handle inputs aswell as outputs... But I'm having some problems decoding 'tuple' types, though.
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.
IMHO we should make it work instead of exposing internals like toGoType
.
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 agree. Let's try to clean up in the abi-swamp
Closing, will reopen in after some rebase |
This is the first PR for the signer, containing some preparations for future work. Note for reviewers: This PR should not affect any existing feature or change any functionality in the way geth works today (so please shout if you believe it does). Contents of this PR:
toGoType
->ToGoType
: to enable parsing calldata according to an ABI-specification.common.Address
in e.g.to
- field insendTransaction
, and will internally remember what the input was, instead of storing only the 20 bytes that make up the address. This will make it possible for geth to forward the data verbatim, and the UI can alert the user about erroneous checksums. Also, the signer API uses this already.Origin
), and don't like to hardcode every possible value into the http server.