-
Notifications
You must be signed in to change notification settings - Fork 21.4k
internal/ethapi: fix tx.from in eth_simulateV1 #31480
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
Conversation
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.
Looks good to me overall but I think it would make sense to expand the test a little bit. Have a look at the comment below.
| } | ||
| require.Len(t, summary, 1, "expected 1 transaction in simulated block") | ||
| require.Len(t, summary[0].Transactions, 1, "expected 1 transaction in simulated block") | ||
| require.Equal(t, sender, summary[0].Transactions[0].From, "sender address mismatch") |
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're only having a single sender and a single transaction, it would make sense to have a few and check that the order is correct, lest they get mixed up somehow.
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 have now address this comment, sorry for the delay.
Issue statement: when user requests eth_simulateV1 to return full transaction objects, these objects always had an empty `from` field. The reason is we lose the sender when translation the message into a types.Transaction which is then later on serialized. I did think of an alternative but opted to keep with this approach as it keeps complexity at the edge. The alternative would be to pass down a signer object to RPCMarshal* methods and define a custom signer which keeps the senders in its state and doesn't attempt the signature recovery.
Issue statement: when user requests eth_simulateV1 to return full transaction objects, these objects always had an empty `from` field. The reason is we lose the sender when translation the message into a types.Transaction which is then later on serialized. I did think of an alternative but opted to keep with this approach as it keeps complexity at the edge. The alternative would be to pass down a signer object to RPCMarshal* methods and define a custom signer which keeps the senders in its state and doesn't attempt the signature recovery.
Issue statement: when user requests eth_simulateV1 to return full transaction objects, these objects always had an empty `from` field. The reason is we lose the sender when translation the message into a types.Transaction which is then later on serialized. I did think of an alternative but opted to keep with this approach as it keeps complexity at the edge. The alternative would be to pass down a signer object to RPCMarshal* methods and define a custom signer which keeps the senders in its state and doesn't attempt the signature recovery.
Issue statement: when user requests eth_simulateV1 to return full transaction objects, these objects always had an empty
fromfield. The reason is we lose the sender when translation the message into a types.Transaction which is then later on serialized.I did think of an alternative but opted to keep with this approach as it keeps complexity at the edge. The alternative would be to pass down a signer object to RPCMarshal* methods and define a custom signer which keeps the senders in its state and doesn't attempt the signature recovery.