feat: support withdraws, calls and reverts with Gateway contract by lumtis · Pull Request #2666 · zeta-chain/node

lumtis · 2024-08-09T06:24:32Z

Description

Complete most tasks listed in #2552 outside of unit tests

coderabbitai · 2024-08-09T06:24:39Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Base branches to auto review (1)

develop

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai generate interesting stats about this repository and render them as a table.
- @coderabbitai show all the console.log statements in this repository.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

github-actions · 2024-08-11T15:06:00Z

!!!WARNING!!!
nosec detected in the following files: x/crosschain/keeper/cctx_orchestrator_validate_outbound.go

Be very careful about using #nosec in code. It can be a quick way to suppress security warnings and move forward with development, it should be employed with caution. Suppressing warnings with #nosec can hide potentially serious vulnerabilities. Only use #nosec when you're absolutely certain that the security issue is either a false positive or has been mitigated in another way.

Only suppress a single rule (or a specific set of rules) within a section of code, while continuing to scan for other problems. To do this, you can list the rule(s) to be suppressed within the #nosec annotation, e.g: /* #nosec G401 */ or //#nosec G201 G202 G203
Broad #nosec annotations should be avoided, as they can hide other vulnerabilities. The CI will block you from merging this PR until you remove #nosec annotations that do not target specific rules.

Pay extra attention to the way #nosec is being used in the files listed above.

proto/zetachain/zetacore/crosschain/cross_chain_tx.proto

proto/zetachain/zetacore/pkg/coin/coin.proto

x/crosschain/keeper/cctx_orchestrator_validate_outbound.go

fbac · 2024-08-12T15:56:43Z

x/crosschain/keeper/evm_hooks.go

-		if errZrc20 != nil && errZetaSent != nil {
-			// This log does not contain any of the two events
-			continue
+		if !crypto.IsEmptyAddress(gatewayAddr) {


Is this the only way of discovering an inbound is V2?

It would be worth it to create a protocol helper function isV2transaction or similar, which gathers all the logic of V2 discovery, so we don't rely on this "deep protocol knowledge" which can be forgotten.

v1 and v2 events will be both parsed sequentially but for v2 it can only happen if the gateway is defined in the protocol otherwise events can't be parsed

fbac · 2024-08-12T16:36:09Z

x/crosschain/keeper/evm_hooks.go

+	if errZrc20 != nil && errZetaSent != nil {
+		// This log does not contain any of the two events
+		return nil
+	}


We check here that the two errors are not nil at the same time, but we never check them independently.

What would happen if errZrc20 OR errZetaSent carries an error?

I copied existing logic, I agree it seems that it should be improved, but in practice nothing should happen because if error is non nil, the event will be nil, then the associated even will be skipped.

This code will be removed once we migrate the codebase to only parse v2 events

e2e/runner/v2_setup_zeta.go

x/fungible/keeper/v2_evm.go

zetaclient/chains/evm/observer/v2_outbound.go

e2e/e2etests/test_operation_add_liquidity_eth.go

skosito · 2024-08-14T15:10:42Z

e2e/e2etests/test_v2_erc20_deposit_and_call_revert.go

+	r.Logger.CCTX(*cctx, "deposit")
+	require.Equal(r, crosschaintypes.CctxStatus_Reverted, cctx.CctxStatus.Status)
+
+	// check the balance is more than 0


is there a way to know how much more than 0 this is?

Doing the calculation here of the fees but I think it is fine like this here as a E2E test, the account is initialize with 0, we want to check it receives tokens

e2e/e2etests/test_v2_erc20_deposit_and_call_revert_with_call.go

e2e/e2etests/test_v2_eth_deposit_and_call.go

e2e/runner/evm.go

x/crosschain/keeper/cctx_orchestrator_validate_outbound.go

x/crosschain/keeper/evm_hooks.go

x/crosschain/keeper/gas_payment.go

zetaclient/chains/evm/observer/outbound_test.go

x/fungible/keeper/v2_deposits.go

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

swift1337

Did a first round of reviews, mostly zetaclient. Look good, let's some comments

zetaclient/chains/evm/cctx.go

zetaclient/chains/evm/observer/outbound.go

zetaclient/chains/evm/observer/v2_inbound.go

zetaclient/chains/evm/observer/v2_outbound.go

zetaclient/zetacore/client_monitor.go

zetaclient/chains/evm/signer/v2_sign.go

zetaclient/chains/evm/signer/v2_signer.go

lumtis · 2024-08-15T14:56:18Z

@fbac ff75b88
@skosito 9cdcfa9
@swift1337 b28089c

* update protocol contracts package * update config and runner * init contract deploy * add erc1967proxy in contracts * fix gateway evm deploy * zevm setuip * format * update version * update v1 import path * reorganize import v2 * fix import * update new version * add custody setup * feat(E2E): add body for smart contract V2 tests (#2609) * add runner for v2 * implement helper function * add test bodies * add makefile entry * import * update system contracts * feat: implement gas token deposit with protocol contract v2 (#2616) * add protocol contract version in cctx * read deposit from ZetaClient * deposit gas token * run deposit * add tests * add event check * add workflow for test * put error handling higher level ObserverGateway * add named interface * feat: withdraw SOL from ZEVM to Solana (#2560) * port Panruo's outbound code and make compile pass * make SOL withdraw e2e test passing * make solana outbound tracker goroutine working * allow solana gateway address to update * integrate sub methods of SignMsgWithdraw and SignWithdrawTx * initiate solana outbound tracker reporter * implemented solana outbound tx verification * use the amount in tx result for outbound vote * post Solana priority fee to zetacore * config Solana fee payer private key * resolve 1st wave of comments in PR review * resolve 2nd wave of comments * refactor IsOutboundProcessed as VoteOutboundIfConfirmed; move outbound tracker iteration logic into ProcessOutboundTrackers sub method * resolve 3rd wave of PR feedback * added description to explain what do we do about the outbound tracker txHash * add additional error message; add additional method comment * fix gosec err * replace contex.TODO() with context.Background() * feat: detect memo in btc txn from OP_RETURN and inscription (#2533) * parse inscription like witness data * more comment * remove unused code * parse inscription * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * pull origin * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> * review feedbacks * update review feedbacks * add mainnet txn * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * parse inscription like witness data * more comment * remove unused code * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> * Update zetaclient/chains/bitcoin/tx_script.go Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * pull origin * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> * review feedbacks * update review feedbacks * update make generate * fix linter * remove over flow * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * Update zetaclient/chains/bitcoin/tokenizer.go Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * Update zetaclient/chains/bitcoin/tokenizer.go Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * Update zetaclient/chains/bitcoin/tokenizer.go Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * Update zetaclient/chains/bitcoin/tokenizer.go Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * update review feedback * update code commnet * update comment * more comments * Update changelog.md * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * Update zetaclient/chains/bitcoin/observer/inbound.go Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * clean up * format code --------- Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * refactor(zetaclient)!: improve AppContext (#2568) * Implement chain registry * Rewrite test-cases for AppContext * Drop `supplychecker` * Refactor app ctx Update worker * Refactor orchestrator * Refactor observer&signer; DROP postBlockHeaders * Fix test cases [1] * Update changelog * Allow Zeta Chain in appContext; address PR comments [1] * Fix app context update * Check for `chain.IsZeta()` * Add AppContext.FilterChains * Fix test cases [2] * Fix test cases [3] * Address PR comments [1] * Address PR comments [2] * Add tests for `slices` * Fix e2e tests [1] * Fix e2e tests [2] * Resolve conflicts, converge codebase between PRs * Add lodash; remove slices pkg * Address PR comments * Minor logging fix * Address PR comments * fix(`crosschain`): set sender for ERC20 whitelist admin CCTX inbound (#2631) * fix whitelist sender * add E2E test * fix(ci): Update golang cross compile to 1.22.4 (#2635) * Update golang cross compile to 1.22.4 * update deprecated --skip-validate --skip-release flags * Added goreleaser check (#2636) * update solidity version * feat: integrate Smart Contract V2 `depositAndCall` for ETH and ERC20 (#2639) * add testdapp v2 * update ZRC20 version * initialize deposit and call * add deposit and call * implement E2E test * move contracts * fix evm deploy ZRC20 * deposit and call local test * complete deposit and call test * add support for E2E tests * comments * fix unit tests * feat: support withdraws, calls and reverts with Gateway contract (#2666) * add v2 zevm inbound * refactor abi in signers * outbound for gas withdraw * fix withdraw * gateway execute parse * add support for withdraw and call * erc20 withdraw and withdraw and call * implement erc20 withdraw e2e tests * test withdraw and call * reading inbound for no assset call * simple call implementation * initialize revert * fix the tests * upgrade smart contracts to latest version * small fix smart contract call * revert tests skeleton * fix test * implement revert tests * add revert support * fix revert tests * make generate * add support for revert gas limit * fix tests * add liquidity test * fix liquidity test * Update proto/zetachain/zetacore/pkg/coin/coin.proto Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * Update x/crosschain/types/revert_options.go Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * Update x/crosschain/types/revert_options.go Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * Borja comments * Stefan comments * dmitry review * fix unit tests --------- Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> * changelogs * fix unit test * tentative fix for upgrade * fix gateway zero address * fix admin test * add back admin tests * v2 conditional matric --------- Co-authored-by: Charlie Chen <34498985+ws4charlie@users.noreply.github.com> Co-authored-by: dev-bitSmiley <153714963+bitSmiley@users.noreply.github.com> Co-authored-by: Dmitry S <11892559+swift1337@users.noreply.github.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev> Co-authored-by: Christopher Fuka <97121270+CryptoFewka@users.noreply.github.com> Co-authored-by: Charlie <31941002+CharlieMc0@users.noreply.github.com>

lumtis added 3 commits August 8, 2024 13:59

add v2 zevm inbound

634dd91

refactor abi in signers

1e08033

outbound for gas withdraw

99cdcf5

github-actions bot added ci Changes to CI pipeline or github actions breaking:proto breaking:cli labels Aug 9, 2024

lumtis changed the base branch from develop to feat/contract-v2-2 August 9, 2024 06:24

github-actions bot removed ci Changes to CI pipeline or github actions breaking:proto labels Aug 9, 2024

lumtis added 8 commits August 9, 2024 09:48

fix withdraw

5340077

gateway execute parse

3d4b03c

add support for withdraw and call

1be6f6f

erc20 withdraw and withdraw and call

0ac8d95

implement erc20 withdraw e2e tests

f2288b7

test withdraw and call

a5ec5ab

reading inbound for no assset call

ec88dd9

simple call implementation

17e084e

github-actions bot added the breaking:proto label Aug 10, 2024

lumtis changed the title ~~feat: support withdraw with Gateway contract~~ feat: support withdraws and calls with Gateway contract Aug 10, 2024

initialize revert

4a977fb

github-actions bot added the nosec label Aug 11, 2024

lumtis changed the title ~~feat: support withdraws and calls with Gateway contract~~ feat: support withdraws, calls and reverts with Gateway contract Aug 12, 2024

fbac reviewed Aug 13, 2024

View reviewed changes

lumtis added 5 commits August 13, 2024 11:02

fix the tests

f5e3ce9

upgrade smart contracts to latest version

0df1c6d

small fix smart contract call

052bcb2

revert tests skeleton

8d1f0f0

fix test

0da87ab

lumtis requested review from kingpinXD, skosito, swift1337 and ws4charlie as code owners August 14, 2024 14:38

skosito reviewed Aug 14, 2024

View reviewed changes

lumtis added 4 commits August 15, 2024 11:18

add support for revert gas limit

5ff7d21

fix tests

6621fe7

add liquidity test

cc3eb22

fix liquidity test

fc67bba

This was linked to issues Aug 15, 2024

Add E2E test to give liquidity to Uniswap poop #2676

Closed

Add support for custom gas limit for V2 revert outbounds to external chains #2699

Closed

This was unlinked from issues Aug 15, 2024

Add E2E test to give liquidity to Uniswap poop #2676

Closed

Add support for custom gas limit for V2 revert outbounds to external chains #2699

Closed

lumtis and others added 3 commits August 15, 2024 14:42

Update proto/zetachain/zetacore/pkg/coin/coin.proto

02e04be

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

Update x/crosschain/types/revert_options.go

f90bfc4

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

Update x/crosschain/types/revert_options.go

113258a

Co-authored-by: Francisco de Borja Aranda Castillejo <me@fbac.dev>

swift1337 reviewed Aug 15, 2024

View reviewed changes

lumtis added 3 commits August 15, 2024 16:11

Borja comments

ff75b88

Stefan comments

9cdcfa9

dmitry review

b28089c

lumtis requested review from fbac, skosito and swift1337 August 15, 2024 14:56

skosito approved these changes Aug 15, 2024

View reviewed changes

fix unit tests

c5868af

fbac approved these changes Aug 16, 2024

View reviewed changes

lumtis merged commit e900a96 into feat/contract-v2-2 Aug 16, 2024

lumtis deleted the feat/v2-withdraws branch August 16, 2024 10:58

Conversation

lumtis commented Aug 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

coderabbitai bot commented Aug 9, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Chat

CodeRabbit Commands (invoked as PR comments)

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

github-actions bot commented Aug 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fbac Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

lumtis Aug 15, 2024

Choose a reason for hiding this comment

Uh oh!

fbac Aug 12, 2024

Choose a reason for hiding this comment

Uh oh!

lumtis Aug 15, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

skosito Aug 14, 2024

Choose a reason for hiding this comment

Uh oh!

lumtis Aug 15, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

swift1337 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

lumtis commented Aug 15, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

lumtis commented Aug 9, 2024 •

edited

Loading

coderabbitai bot commented Aug 9, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

github-actions bot commented Aug 11, 2024 •

edited

Loading