-
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
accounts/abi/bind: fix NewFallback test #22053
Conversation
Hey @yoomee1313 thanks for finding this. It is indeed a bug that diff --git a/accounts/abi/bind/bind_test.go b/accounts/abi/bind/bind_test.go
index 4a504516b..72e6505a4 100644
--- a/accounts/abi/bind/bind_test.go
+++ b/accounts/abi/bind/bind_test.go
@@ -1701,9 +1701,12 @@ var bindTests = []struct {
}
// Test fallback function
+ gotEvent = false
opts.Value = nil
calldata := []byte{0x01, 0x02, 0x03}
- c.Fallback(opts, calldata)
+ if _, err := c.Fallback(opts, calldata); err != nil {
+ t.Fatalf("Failed to call fallback: %v", err)
+ } The call to the fallback function fails with the following error:
which indicates that the tx always fails. When running the contract with remix the call to fallback executes correctly (with slightly different calldata), emits the correct events with 26598 gas used (so well within the 10 mil allowance). I'm sure that the calldata is just malformed. I'd rather fix the calldata instead of removing the calldatacopy call |
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.
After some more consideration, I think this PR is good the way it is.
I will keep investigating the issue with calldatacopy but our tests shouldn't break because of it.
Thanks for your work @yoomee1313
fix Newfallback test
related issue #22049