-
Notifications
You must be signed in to change notification settings - Fork 105
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
fix: support depositAndCall
and withdrawAndCall
with empty message
#3018
base: develop
Are you sure you want to change the base?
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3018 +/- ##
===========================================
- Coverage 66.25% 64.48% -1.78%
===========================================
Files 406 409 +3
Lines 22772 28630 +5858
===========================================
+ Hits 15088 18462 +3374
- Misses 6907 9373 +2466
- Partials 777 795 +18
|
@@ -52,6 +52,7 @@ | |||
* [2853](https://github.com/zeta-chain/node/pull/2853) - calling precompile through sc with sc state update | |||
* [2925](https://github.com/zeta-chain/node/pull/2925) - add recover to init chainer to diplay informative message when starting a node from block 1 | |||
* [2909](https://github.com/zeta-chain/node/pull/2909) - add legacy messages back to codec for querier backward compatibility | |||
* [3018](https://github.com/zeta-chain/node/pull/3018) - support `DepositAndCall` and `WithdrawAndCall` with empty payload |
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.
shouldnt this be DepositedAndCalled and WithdrawedAndCalled?
@@ -190,7 +199,7 @@ func (r *E2ERunner) V2ZEVMToEMVAuthenticatedCall( | |||
return tx | |||
} | |||
|
|||
// V2ZEVMToEMVCall calls authenticated Call of Gateway on ZEVM through contract |
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.
also did some renaming on these tests in my PR
|
||
// this field describes if a smart contract call should be made for a inbound | ||
// with assets only used for protocol contract version 2 | ||
bool is_cross_chain_call = 12; |
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.
not sure about this name, especially because it is i crosschain proto, maybe can reflect assets in the name?
if we need to keep it, maybe we can add a bit more in comment, and make comment same for proto below
types.ProtocolContractVersion_V2, | ||
event.CallOptions.IsArbitraryCall, | ||
types.WithZEVMRevertOptions(event.RevertOptions), | ||
types.WithCrossChainCall(true), |
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 this case its true no matter of message lenght right? maybe we can add comment
Description
Currently both would be interpreted as normal dpeosit or withdraw
This PR fix this with using a new field in the inbound and a new type of event in the gateway contracts
Smart contract associated PR: zeta-chain/protocol-contracts#403