-
Notifications
You must be signed in to change notification settings - Fork 379
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
Support blob transactions in data poster external signer #2169
Conversation
|
||
type SignTxArgs struct { | ||
*apitypes.SendTxArgs |
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 was looking to see why the blob fields weren't on apitypes.SendTxArgs
already, and looking at the comments on apitypes.SendTxArgs
it says
// This struct is identical to ethapi.TransactionArgs, except for the usage of
// common.MixedcaseAddress in From and To
but the fields haven't been copied over yet from ethapi.TransactionArgs
. This looks like a typical case of code that's supposed to be kept in sync manually being overlooked. For the time being, in ethapi.TransactionArgs
the fields actually have different types than the types you specified here, so until it's fixed upstream we should probably use the types they already picked.
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 raised ethereum/go-ethereum#29161 with upstream to see if this is just an oversight or something more.
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.
Makes sense, changed to follow types of fields in ethapi.TransactionArgs struct.
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
No description provided.