Skip to content

Conversation

@kingpinXD
Copy link
Member

@kingpinXD kingpinXD commented Dec 10, 2025

Description

  • Adds a flag to dynamically switch ZETA V2 Flows on and off .
  • Updates E2E tests to use the flag to verify behaviour.
Flow Behavior User Funds Track Transaction
V2 ZETA Deposit SetAbort() - CCTX aborted, no processing ZETA locked in connector on external chain Query CCTX by inbound hash to see error message and aborted status
V2 ZETA Withdrawal Transaction reverts User keeps ZETA on ZetaChain No on-chain record exists; zetacore logs contain error details
  • Note: Aborted transactions can be manually refunded, if needed. I chose not to do it automatically, as an automatic refund means a deposit to a zEVM address. Just uses the abort address instead of the receiver

How Has This Been Tested?

  • Tested CCTX in localnet
  • Tested in development environment
  • Go unit tests
  • Go integration tests
  • Tested via GitHub Actions

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 10, 2025

Important

Review skipped

Draft detected.

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.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

switch {
case zrc20 == wzetaContractAddress:
inboundDetails, err = k.getZETAInboundDetails(ctx, receiverChainID, callOptions)
return types.ErrZetaThroughGateway
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is slightly different from the inbound option; the inbound causes the cctx to be in an aborted state, but this error prevents the cctx from being created.

We can abort the cctx here as well, bit this approach seems cleaner,

It would be easier to modify the inbound logic to just ignore the tx instead of aborting it (In case we want similar behaviour for both )

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it depends on how will it look for users, and how easily they can get information why their cctx is aborted or didnt go through in this case

what is the difference from that point of view do you have example?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For deposits, the inbound hash would end up creating an aborted CCTX with the error message

	utils.RequireCCTXStatus(r, cctx, crosschaintypes.CctxStatus_Aborted)
	require.Equal(r, cctx.CctxStatus.StatusMessage, crosschaintypes.ErrZetaThroughGateway.Error())

For withdrawals created from zEVM no cctx is created

	utils.EnsureNoCctxMinedByInboundHash(r.Ctx, tx.Hash().Hex(), r.CctxClient)

Copy link
Member Author

@kingpinXD kingpinXD Dec 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing a withdraw does emit an event
https://github.com/zeta-chain/zeta-node/blob/88f4df439231873b70352b6b4ae9fbf9cfe4c468/x/crosschain/keeper/evm_hooks.go#L98-L98
I think this was added to track such failied withdraws , however the logic does not work , since after emitting the event we are returning an error ,

This essentially rolls back the logs
Code from ApplyTransaction

		if err = k.PostTxProcessing(tmpCtx, signerAddr, *msg, receipt); err != nil {
			// If hooks returns an error, revert the whole tx.
			res.VmError = errorsmod.Wrap(err, "failed to execute post transaction processing").Error()
			k.Logger(ctx).Error("tx post processing failed", "error", err)
			// If the tx failed in post processing hooks, we should clear all log-related data
			// to match EVM behavior where transaction reverts clear all effects including logs
			res.Logs = nil
			receipt.Logs = nil
			receipt.Bloom = ethtypes.Bloom{} // Clear bloom filter
		} else if commitFn != nil {

The only way to persist this event would be to not revert the tx, essentially just return nil

	case zrc20 == wzetaContractAddress:
			// EmitEvent from here 
			return nil

But that would cause funds to get stuck on the fungible module address

Flow Behavior User Funds Track Transaction
V2 ZETA Deposit SetAbort() - CCTX aborted, no processing ZETA locked in connector on external chain Query CCTX by inbound hash to see error message and aborted status
V2 ZETA Withdrawal Transaction reverts User keeps ZETA on ZetaChain No on-chain record exists; zetacore logs contain error details

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

makes sense, whatever you see as best fit, if we cant make same behavior without big refactor i think what you did initially is fine

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think saving a withdrawal to the state is not a small refactor, as the EVM hook rolls back any TX that returns an error. Rolling back is also good as there is no loss of funds

As for deposits, we have two options: revert or abort. The issue with revert is that even if we revert it, it still goes through the V2 flow. Aborting is safe and also allows us to refund the user using MsgRefundAborted if needed.

@kingpinXD kingpinXD added UPGRADE_TESTS Run make start-upgrade-tests V2_CONNECTOR_MIGRATION_TESTS ADMIN_UPGRADE_TESTS Run make start-upgrade-test-admin labels Dec 23, 2025
@codecov
Copy link

codecov bot commented Dec 23, 2025

Codecov Report

❌ Patch coverage is 48.21429% with 29 lines in your changes missing coverage. Please review.
✅ Project coverage is 65.89%. Comparing base (8df6b33) to head (be3fae4).

Files with missing lines Patch % Lines
x/observer/types/message_update_v2_zeta_flows.go 0.00% 20 Missing ⚠️
x/crosschain/keeper/cctx_gateway_zevm.go 28.57% 4 Missing and 1 partial ⚠️
x/crosschain/keeper/v2_zevm_inbound.go 66.66% 2 Missing and 2 partials ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4486      +/-   ##
==========================================
- Coverage   65.93%   65.89%   -0.05%     
==========================================
  Files         451      453       +2     
  Lines       27251    27299      +48     
==========================================
+ Hits        17969    17988      +19     
- Misses       8310     8336      +26     
- Partials      972      975       +3     
Files with missing lines Coverage Δ
x/authority/types/authorization_list.go 100.00% <ø> (ø)
x/observer/keeper/crosschain_flags.go 91.42% <100.00%> (+1.42%) ⬆️
...observer/keeper/msg_server_update_v2_zeta_flows.go 100.00% <100.00%> (ø)
x/observer/types/crosschain_flags.go 100.00% <100.00%> (ø)
x/crosschain/keeper/v2_zevm_inbound.go 75.00% <66.66%> (-1.79%) ⬇️
x/crosschain/keeper/cctx_gateway_zevm.go 88.09% <28.57%> (-11.91%) ⬇️
x/observer/types/message_update_v2_zeta_flows.go 0.00% <0.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kingpinXD kingpinXD added the LEGACY_TESTS Run make start-legacy-test label Dec 23, 2025
@kingpinXD kingpinXD changed the base branch from main to develop January 3, 2026 10:21
@github-actions github-actions bot added the ci Changes to CI pipeline or github actions label Jan 3, 2026
@kingpinXD kingpinXD changed the base branch from develop to main January 3, 2026 10:21
@github-actions github-actions bot removed the ci Changes to CI pipeline or github actions label Jan 3, 2026
@kingpinXD kingpinXD closed this Jan 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ADMIN_UPGRADE_TESTS Run make start-upgrade-test-admin breaking:cli breaking:proto LEGACY_TESTS Run make start-legacy-test UPGRADE_TESTS Run make start-upgrade-tests V2_CONNECTOR_MIGRATION_TESTS

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants