test: make Bitcoin inscription based depositAndCall compatible for live network by ws4charlie · Pull Request #4040 · zeta-chain/node

ws4charlie · 2025-07-22T21:02:16Z

Description

The changes in this PR:

Reuse test_dapp_v2 for Bitcoin deposit and call related E2E tests.
Compute depositor fee so as to check deposited zrc20 BTC token amount into ZEVM.
Fix randomPayloadWithSize to return exact length (both odd and even) of payload.
Add helper functions to help the E2E tests to with live networks.
- SelectUTXOs
- WaitForTxInclusion
- CalcReceivedAmount
- EstimateFeeRate

Closes: #4031

How Has This Been Tested?

Summary by CodeRabbit

Tests
- Improved Bitcoin and Ethereum end-to-end tests for deposit, withdrawal, and payload handling, including dynamic payload sizing, balance verification, and enhanced transaction confirmation.
- Increased maximum payload sizes in ETH deposit and withdraw tests to match gateway limits.
- Added synchronization step for Bitcoin withdrawal tests to ensure transaction confirmation.
- Updated test logic to use predefined DApp addresses and more robust validation of transaction outcomes.
Chores
- Updated changelog to reflect recent changes and test improvements.
Bug Fixes
- Enhanced Bitcoin node interaction to avoid unnecessary queries when the node is not ready.

… E2E test

coderabbitai · 2025-07-22T21:02:25Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

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.

📝 Walkthrough

Walkthrough

This update refactors several Bitcoin and Ethereum end-to-end (E2E) tests, focusing on improved payload handling, dynamic fee estimation, and more robust transaction confirmation logic. The Bitcoin E2E runner’s transaction confirmation is now polling-based, and test contracts are replaced with predefined DApp addresses. Changelog and test documentation are updated accordingly.

Changes

File(s)	Change Summary
changelog.md	Removed entry for PR #4004; added test entry for PR #4040 in UNRELEASED section.
e2e/e2etests/e2etests.go	Removed "fee rate" argument from a specific Bitcoin inscription test case.
e2e/e2etests/helpers.go	`randomPayload` now generates 100-char payloads; `randomPayloadWithSize` reworked for precise hex string sizing and better edge handling.
e2e/e2etests/test_bitcoin_deposit_call.go	Refactored test to use predefined DApp address, improved payload sizing, and balance checks.
e2e/e2etests/test_bitcoin_std_deposit_and_call.go	Refactored to use fixed test DApp address, dynamic payloads, and actual received amount calculations.
e2e/e2etests/test_bitcoin_std_memo_inscribed_deposit_and_call.go	Simplified inputs, dynamic fee estimation, replaced contract deployment with fixed DApp address, and improved payload testing.
e2e/e2etests/test_bitcoin_withdraw_rbf.go	Added synchronization after withdrawal initiation to ensure CCTX creation before proceeding.
e2e/e2etests/test_eth_deposit_and_call_big_payload.go e2e/e2etests/test_eth_withdraw_and_call_big_payload.go	Increased ETH test payload size from 1440 to 2880 bytes to match gateway limits.
e2e/runner/bitcoin.go	Refactored transaction confirmation to polling-based logic; added fee estimation and received amount calculation utilities; updated method signatures.
zetaclient/chains/bitcoin/observer/utxos.go	Early exit added in UTXO fetch if Bitcoin node is not ready.

Sequence Diagram(s)

sequenceDiagram
    participant Test as E2E Test
    participant Runner as E2ERunner
    participant BTCNode as Bitcoin Node
    participant ZEVM as ZEVM Chain

    Test->>Runner: Initiate BTC deposit/inscription with memo
    Runner->>BTCNode: Send BTC transaction
    Runner->>BTCNode: WaitForTxInclusion (polling)
    BTCNode-->>Runner: Return TxRawResult after inclusion
    Runner->>ZEVM: Wait for CCTX mined
    ZEVM-->>Runner: Confirm DApp called with payload and amount
    Runner-->>Test: Assert balance and payload delivery

Estimated code review effort

4 (~90 minutes)

The changes encompass substantial refactoring in the E2E Bitcoin runner, significant updates to several complex E2E tests, and non-trivial utility enhancements. The review requires careful attention to test correctness, transaction handling logic, and potential side effects across the test suite.

✨ Finishing Touches

🧪 Generate unit tests

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch e2e-make-btc-inscription-deposit-work-for-nightly-test

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

🪧 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>, please review it.
- Explain this complex logic.
- 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 explain this code block.
- @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 gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @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.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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 using 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 generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai generate unit tests to generate unit tests for this 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.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

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.

codecov · 2025-07-22T21:06:27Z

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 64.94%. Comparing base (909ec4e) to head (bdd550b).

Files with missing lines	Patch %	Lines
zetaclient/chains/bitcoin/observer/utxos.go	25.00%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #4040   +/-   ##
========================================
  Coverage    64.94%   64.94%           
========================================
  Files          475      475           
  Lines        35139    35142    +3     
========================================
+ Hits         22821    22823    +2     
  Misses       11245    11245           
- Partials      1073     1074    +1

Files with missing lines	Coverage Δ
zetaclient/chains/bitcoin/observer/utxos.go	`82.20% <25.00%> (-2.15%)`	⬇️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

…-nightly-test

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (4)

e2e/e2etests/helpers.go (2)
20-20: Clarify that the function returns a hex-encoded string.

The comment should explicitly mention that the returned string is hex-encoded to avoid confusion.
-// randomPayloadWithSize generates a random payload string containing exactly 'size' characters
+// randomPayloadWithSize generates a random hex-encoded string containing exactly 'size' characters
27-50: Simplify the implementation for better readability.

The current implementation handles even and odd sizes separately, but this can be simplified while maintaining the same functionality.
-	var (
-		bytes []byte
-		even  = size%2 == 0
-	)
-
-	// the size will double when converting to hex string, so we just need half the size
-	if even {
-		bytes = make([]byte, size/2)
-	} else {
-		// add 1 more byte if size is odd, we trim it later
-		bytes = make([]byte, (size/2)+1)
-	}
-
-	// generate random bytes
+	// hex encoding doubles the size, so we need ceil(size/2) bytes
+	bytes := make([]byte, (size+1)/2)
 	_, err := rand.Read(bytes)
 	require.NoError(r, err)
 
-	// hex encode the bytes, the size is double
-	if even {
-		return hex.EncodeToString(bytes)
-	}
-
-	// trim the last letter to fit the odd size
-	return hex.EncodeToString(bytes)[:size]
+	// hex encode and trim to exact size
+	return hex.EncodeToString(bytes)[:size]
e2e/runner/bitcoin.go (2)
491-491: Fix typo in comment.
-		// error may occue if the tx is not yet included
+		// error may occur if the tx is not yet included
506-509: Consider documenting the mempool acceptance rationale.

The comment mentions accepting 0 confirmations (mempool transactions) is sufficient. While this is reasonable for E2E tests, consider adding a brief explanation about why this is safe in this context.
 		// as far as tx gets into the mempool (0 confirmations), we consider it included
 		// there's no need to wait for 1 block to be able to send out subsequent transactions
+		// This is safe for E2E tests as we control the test environment and transaction ordering

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5eb258 and 27bb3c3.

📒 Files selected for processing (11)

changelog.md (1 hunks)
e2e/e2etests/e2etests.go (0 hunks)
e2e/e2etests/helpers.go (1 hunks)
e2e/e2etests/test_bitcoin_deposit_call.go (2 hunks)
e2e/e2etests/test_bitcoin_std_deposit_and_call.go (1 hunks)
e2e/e2etests/test_bitcoin_std_memo_inscribed_deposit_and_call.go (2 hunks)
e2e/e2etests/test_bitcoin_withdraw_rbf.go (1 hunks)
e2e/e2etests/test_eth_deposit_and_call_big_payload.go (1 hunks)
e2e/e2etests/test_eth_withdraw_and_call_big_payload.go (1 hunks)
e2e/runner/bitcoin.go (4 hunks)
zetaclient/chains/bitcoin/observer/utxos.go (1 hunks)

📓 Path-based instructions (1)

**/*.go

⚙️ CodeRabbit Configuration File

Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.

Files:

e2e/e2etests/test_eth_deposit_and_call_big_payload.go
e2e/e2etests/test_bitcoin_withdraw_rbf.go
e2e/e2etests/test_eth_withdraw_and_call_big_payload.go
zetaclient/chains/bitcoin/observer/utxos.go
e2e/e2etests/helpers.go
e2e/e2etests/test_bitcoin_std_memo_inscribed_deposit_and_call.go
e2e/e2etests/test_bitcoin_std_deposit_and_call.go
e2e/runner/bitcoin.go
e2e/e2etests/test_bitcoin_deposit_call.go

🧠 Learnings (11)

📓 Common learnings

Learnt from: gartnera
PR: zeta-chain/node#3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like `cmd/zetae2e/init.go` in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.

Learnt from: skosito
PR: zeta-chain/node#3939
File: go.mod:52-52
Timestamp: 2025-05-30T16:31:30.275Z
Learning: The ethermint dependency updates in the zeta-chain/node repository are typically moves between feature branches and main branch of the same fork, not breaking API changes. CI status should be verified before assuming compilation issues.

Learnt from: gartnera
PR: zeta-chain/node#3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.

Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/chain_activate.go:184-247
Timestamp: 2024-07-05T00:02:31.446Z
Learning: The `CreateSignerObserverBTC` function in `zetaclient/orchestrator/chain_activate.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go`.

Learnt from: ws4charlie
PR: zeta-chain/node#2899
File: zetaclient/chains/bitcoin/observer/inbound.go:366-371
Timestamp: 2024-09-19T18:28:58.296Z
Learning: The `bitcoin.CalcDepositorFee` function is covered by live tests.

Learnt from: ws4charlie
PR: zeta-chain/node#2870
File: zetaclient/orchestrator/orchestrator.go:413-413
Timestamp: 2024-09-13T22:29:09.747Z
Learning: When a method is renamed (e.g., `IsUTXO()` to `IsBitcoin()`), and existing tests cover the affected code paths, do not request additional tests for the renamed methods.

Learnt from: gartnera
PR: zeta-chain/node#3228
File: zetaclient/orchestrator/orchestrator.go:388-401
Timestamp: 2024-11-27T22:01:49.732Z
Learning: When reviewing code changes in `zetaclient/orchestrator/orchestrator.go`, avoid suggesting refactoring that is unrelated to the current PR.

Learnt from: ws4charlie
PR: zeta-chain/node#4007
File: e2e/runner/sui.go:237-274
Timestamp: 2025-07-14T21:08:08.725Z
Learning: In test code for the ZetaChain project, it's acceptable to use constants for EVM and Sui address lengths rather than adding complex validation logic. This applies to functions like payload construction in E2E tests where the expected lengths are well-defined and documented.

Learnt from: lumtis
PR: zeta-chain/node#2459
File: e2e/config/config.go:285-293
Timestamp: 2024-07-11T13:10:39.153Z
Learning: Tests should not be suggested in the `e2e` package for the `zeta-chain/node` repository.

Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/orchestrator.go:192-217
Timestamp: 2024-07-04T23:46:38.428Z
Learning: The `GetUpdatedSigner` method in `zetaclient/orchestrator/orchestrator.go` is covered by unit tests in `zetaclient/orchestrator/chain_activate_test.go` and `zetaclient/orchestrator/orchestrator_test.go`.

Learnt from: kingpinXD
PR: zeta-chain/node#3503
File: e2e/runner/require.go:0-0
Timestamp: 2025-02-07T19:33:35.631Z
Learning: In e2e tests for the zeta-chain/node repository, minimize computation and avoid redundant checks that are already covered by unit tests to ensure faster execution. For example, ballot sorting is verified in unit tests, so e2e tests can safely assume the order.

Learnt from: gartnera
PR: zeta-chain/node#3079
File: e2e/e2etests/test_pause_erc20_custody.go:65-68
Timestamp: 2024-11-01T17:07:59.584Z
Learning: In the file `e2e/e2etests/test_pause_erc20_custody.go`, prefer verbosity and avoid extracting common event handling logic into helper functions to enhance readability and ease future refactoring.

Learnt from: ws4charlie
PR: zeta-chain/node#2411
File: zetaclient/orchestrator/orchestrator.go:222-237
Timestamp: 2024-07-04T23:47:56.072Z
Learning: The `GetUpdatedChainObserver` method in the `Orchestrator` class is covered by unit tests in `zetaclient/orchestrator/orchestrator_test.go` and `zetaclient/orchestrator/chain_activate_test.go`.

e2e/e2etests/test_eth_deposit_and_call_big_payload.go (9)

Learnt from: gartnera
PR: #3079
File: e2e/e2etests/test_pause_erc20_custody.go:65-68
Timestamp: 2024-11-01T17:07:59.584Z
Learning: In the file e2e/e2etests/test_pause_erc20_custody.go, prefer verbosity and avoid extracting common event handling logic into helper functions to enhance readability and ease future refactoring.

Learnt from: ws4charlie
PR: #4007
File: e2e/runner/sui.go:237-274
Timestamp: 2025-07-14T21:08:08.725Z
Learning: In test code for the ZetaChain project, it's acceptable to use constants for EVM and Sui address lengths rather than adding complex validation logic. This applies to functions like payload construction in E2E tests where the expected lengths are well-defined and documented.

Learnt from: gartnera
PR: #3071
File: e2e/e2etests/test_stress_eth_withdraw.go:0-0
Timestamp: 2024-10-31T16:21:47.362Z
Learning: In e2e/e2etests/test_stress_eth_withdraw.go, when handling the error returned by stats.Describe in the TestStressEtherWithdraw function, log the error instead of failing the test.

Learnt from: ws4charlie
PR: #2987
File: pkg/memo/codec_abi.go:0-0
Timestamp: 2024-10-10T18:50:23.406Z
Learning: The constant ABIAlignment has been moved from pkg/memo/codec_abi.go to the testutil package.

Learnt from: gartnera
PR: #3071
File: e2e/e2etests/test_stress_eth_withdraw.go:41-42
Timestamp: 2024-10-31T16:23:17.250Z
Learning: In e2e/e2etests/test_stress_eth_withdraw.go, since some goroutines may fail, we should use a mutex to append durations of successful withdrawals for accurate latency reporting.

Learnt from: ws4charlie
PR: #2987
File: testutil/sample/memo.go:0-0
Timestamp: 2024-10-10T18:54:33.554Z
Learning: In the testutil/sample/memo.go file, unchecked type assertions in the ABIPack function are acceptable because it is designed for unit tests and simplicity.

Learnt from: gartnera
PR: #3070
File: cmd/zetae2e/init.go:0-0
Timestamp: 2024-10-30T17:56:16.341Z
Learning: In code reviews for Go files like cmd/zetae2e/init.go in the ZetaChain project, avoid suggesting unrelated refactoring. Focus comments on changes relevant to the PR objectives.

Learnt from: ws4charlie
PR: #3306
File: pkg/math/integer_test.go:0-0
Timestamp: 2025-01-14T03:37:18.430Z
Learning: Use big.Int for handling large number calculations and edge cases to avoid integer overflow issues.

Learnt from: ws4charlie
PR: #2870
File: zetaclient/orchestrator/orchestrator.go:413-413
Timestamp: 2024-09-13T22:29:09.747Z
Learning: When a method is renamed (e.g., IsUTXO() to IsBitcoin()), and existing tests cover the affected code paths, do not request additional tests for the renamed methods.

e2e/e2etests/test_bitcoin_withdraw_rbf.go (11)

Learnt from: gartnera
PR: #3071
File: e2e/e2etests/test_stress_eth_withdraw.go:41-42
Timestamp: 2024-10-31T16:23:17.250Z
Learning: In e2e/e2etests/test_stress_eth_withdraw.go, since some goroutines may fail, we should use a mutex to append durations of successful withdrawals for accurate latency reporting.

Learnt from: ws4charlie
PR: #2870
File: zetaclient/orchestrator/orchestrator.go:413-413
Timestamp: 2024-09-13T22:29:09.747Z
Learning: When a method is renamed (e.g., IsUTXO() to IsBitcoin()), and existing tests cover the affected code paths, do not request additional tests for the renamed methods.

Learnt from: ws4charlie
PR: #2411
File: zetaclient/orchestrator/chain_activate.go:184-247
Timestamp: 2024-07-05T00:02:31.446Z
Learning: The CreateSignerObserverBTC function in zetaclient/orchestrator/chain_activate.go is covered by unit tests in zetaclient/orchestrator/chain_activate_test.go.

Learnt from: ws4charlie
PR: #2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-10-08T15:34:48.217Z
Learning: In the mineTxNSetNonceMark function within bitcoin/observer/outbound_test.go, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.

Learnt from: ws4charlie
PR: #2907
File: zetaclient/chains/bitcoin/observer/outbound_test.go:81-82
Timestamp: 2024-09-23T19:51:43.360Z
Learning: In the mineTxNSetNonceMark function within bitcoin/observer/outbound_test.go, it's acceptable to hardcode the chain ID, as the tests do not require varying chain IDs.

Learnt from: gartnera
PR: #3105
File: e2e/runner/setup_bitcoin.go:51-69
Timestamp: 2024-11-06T21:10:14.301Z
Learning: In test code (e2e/runner/setup_bitcoin.go), adding security measures for private key handling in the GetBtcAddress method is not required.

Learnt from: ws4charlie
PR: #2899
File: zetaclient/chains/bitcoin/observer/inbound.go:366-371
Timestamp: 2024-09-19T18:28:58.296Z
Learning: The bitcoin.CalcDepositorFee function is covered by live tests.

Learnt from: gartnera
PR: #3079
File: e2e/e2etests/test_pause_erc20_custody.go:65-68
Timestamp: 2024-11-01T17:07:59.584Z
Learning: In the file e2e/e2etests/test_pause_erc20_custody.go, prefer verbosity and avoid extracting common event handling logic into helper functions to enhance readability and ease future refactoring.

Learnt from: gartnera
PR: #3632
File: zetaclient/chains/solana/signer/signer.go:304-304
Timestamp: 2025-03-04T22:39:58.395Z
Learning: The Solana signer implementation in zetaclient/chains/solana/signer/signer.go has limited test coverage, particularly for the transaction broadcasting logic with fallback scenarios. Adding this coverage has been acknowledged as a potential future improvement outside the scope of immediate fixes.

Learnt from: gartnera
PR: #3071
File: e2e/e2etests/test_stress_eth_withdraw.go:58-59
Timestamp: 2024-10-31T16:19:26.038Z
Learning: In the Go code within e2e/utils/zetacore.go, the function WaitCctxMinedByInboundHash does not return an error.

Learnt from: ws4charlie
PR: #3306
File: zetaclient/chains/bitcoin/signer/sign.go:225-233
Timestamp: 2025-01-14T03:39:29.764Z
Learning: The signature hash calculation in Bitcoin TSS transactions using txscript.NewCannedPrevOutputFetcher([]byte{}, 0) with correct input amounts has been proven to work reliably in production, despite theoretical concerns about empty previous output fetcher.

e2e/e2etests/test_eth_withdraw_and_call_big_payload.go (10)