-
Notifications
You must be signed in to change notification settings - Fork 1.9k
CCIP-7724 leave from field empty when perform method read #19958
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
base: develop
Are you sure you want to change the base?
Conversation
I see you updated files related to
|
|
return nil, callErr | ||
} | ||
|
||
callMsg := ethereum.CallMsg{ |
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.
can we add a test for that? (also with the comment from JIRA why we do that)
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 added a summary to the PR description.
What would the test look like? We have higher level tests that use method read under the hood.
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.
Judging by the JIRA, it seems essential that the From:
is not set. Hence, it seems natural to enforce this behavior in tests (currently, no test would fail if someone introduces From:
back)
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.
method.GetLatestValue is the only place where From can be set; that's where CallMsg is formed. I agree the test case would be reasonable if the scope of setting CallMsg.From were broader.
p.s. Also it's essential for the From not to be a contract address, we don't necessary need to enforce empty From via tests.
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.
In the tests here
tester := &simpleTester{returnVal: big.NewInt(42), internalType: test} |
CallContract
is called with not a contract address?
JIRA
Execution clients with REVM backend apply EIP-3607 (reject caller with code) even for eth_call's, therefore when to address is a contract, our calls fail.
There is no practical reasons to set from = to address for eth_call's.