test(e2e): cover the merkle batch payment path#82
test(e2e): cover the merkle batch payment path#82mickvandijke merged 2 commits intoWithAutonomi:mainfrom
Conversation
The existing e2e script never exercised payForMerkleTree:
- Default node count was 5, below CANDIDATES_PER_POOL = 16, so
pay_for_merkle_batch returned InsufficientPeers and Auto-mode silently
fell back to per-chunk payForQuotes (which has no pool-count
validation).
- Test files in ugly_files/ are capped at 1 MB, well below the 64-chunk
merkle threshold.
- No step used --merkle, so even with bigger files Auto fallback would
mask any contract-side failure.
Net effect: zero coverage for the merkle path. The recent
WrongPoolCount(16, 8) production revert against odd-depth merkle trees
would not have been caught here.
This adds:
- DEVNET_NODES default bumped from 5 to 16 (+ BOOTSTRAP_COUNT 2 -> 3),
with a comment explaining the threshold. Override via
ANT_TEST_DEVNET_NODES still works.
- New Step 5b that:
* synthesizes a 280 MiB random file (depth-7 chunk band, the exact
production failure mode),
* uploads with --merkle to disable Auto fallback,
* asserts the client's "Submitting merkle batch payment on-chain"
log line is present, so a future silent-fallback regression fails
the test loudly,
* downloads via the saved datamap and SHA256-compares the round
trip.
Override file size with ANT_TEST_MERKLE_FILE_MB; skip the step
entirely with ANT_TEST_SKIP_MERKLE=1 (for disk-constrained CI).
Verified locally on a 16-node Anvil-backed devnet:
PASS: Merkle batch upload (74 chunks, depth=7)
PASS: Merkle batch round-trip (SHA256 match)
There was a problem hiding this comment.
Pull request overview
This PR updates the E2E script to reliably exercise the on-chain merkle batch payment path (payForMerkleTree), which previously had no coverage due to insufficient devnet nodes, small test file sizes, and Auto-mode fallback behavior.
Changes:
- Increase default devnet size to meet the merkle batch peer threshold (16 nodes; bootstrap 3).
- Add a new Step 5b that synthesizes a large file, uploads with
--merkle(disabling Auto fallback), asserts a merkle-specific log line, and verifies round-trip integrity via datamap + SHA256.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if grep -q "Upload complete" "${CLI_STDOUT}" 2>/dev/null; then | ||
| MERKLE_CHUNKS=$(grep "Chunks:" "${CLI_STDOUT}" | head -1 | grep -oE '[0-9]+' | head -1) | ||
| # The fixed client logs "Submitting merkle batch payment on-chain (depth=N)" at info level | ||
| # before calling payForMerkleTree. Confirms we did not hit some other code path | ||
| # (e.g. a silent Auto-mode fallback to per-chunk `payForQuotes`). | ||
| if grep -q "Submitting merkle batch payment on-chain" "${MERKLE_LOG}" 2>/dev/null; then | ||
| MERKLE_DEPTH=$(grep -oE "depth=[0-9]+" "${MERKLE_LOG}" | head -1 | cut -d= -f2) | ||
| pass "Merkle batch upload (${MERKLE_CHUNKS:-?} chunks, depth=${MERKLE_DEPTH:-?})" | ||
| else | ||
| fail "Merkle batch upload" "Upload reported success but merkle log line not found in stderr — possible silent fallback" | ||
| echo " Stderr tail:" | ||
| tail -10 "${MERKLE_LOG}" 2>/dev/null || true | ||
| fi | ||
|
|
There was a problem hiding this comment.
Step 5b can silently do nothing: if the upload succeeds but stdout format changes (or doesn’t contain the literal "Upload complete"), the if grep -q ...; then block is skipped and no PASS/FAIL is recorded. Consider keying success off the upload command’s exit status and explicitly fail when expected markers (e.g., merkle payment log line) are missing, rather than gating the entire test on a stdout substring.
| if grep -q "Upload complete" "${CLI_STDOUT}" 2>/dev/null; then | |
| MERKLE_CHUNKS=$(grep "Chunks:" "${CLI_STDOUT}" | head -1 | grep -oE '[0-9]+' | head -1) | |
| # The fixed client logs "Submitting merkle batch payment on-chain (depth=N)" at info level | |
| # before calling payForMerkleTree. Confirms we did not hit some other code path | |
| # (e.g. a silent Auto-mode fallback to per-chunk `payForQuotes`). | |
| if grep -q "Submitting merkle batch payment on-chain" "${MERKLE_LOG}" 2>/dev/null; then | |
| MERKLE_DEPTH=$(grep -oE "depth=[0-9]+" "${MERKLE_LOG}" | head -1 | cut -d= -f2) | |
| pass "Merkle batch upload (${MERKLE_CHUNKS:-?} chunks, depth=${MERKLE_DEPTH:-?})" | |
| else | |
| fail "Merkle batch upload" "Upload reported success but merkle log line not found in stderr — possible silent fallback" | |
| echo " Stderr tail:" | |
| tail -10 "${MERKLE_LOG}" 2>/dev/null || true | |
| fi | |
| UPLOAD_MARKER_FOUND=false | |
| MERKLE_MARKER_FOUND=false | |
| if grep -q "Upload complete" "${CLI_STDOUT}" 2>/dev/null; then | |
| UPLOAD_MARKER_FOUND=true | |
| MERKLE_CHUNKS=$(grep "Chunks:" "${CLI_STDOUT}" | head -1 | grep -oE '[0-9]+' | head -1) | |
| else | |
| fail "Merkle batch upload" "Upload command succeeded but expected completion marker was not found in stdout" | |
| echo " Stdout tail:" | |
| tail -10 "${CLI_STDOUT}" 2>/dev/null || true | |
| fi | |
| # The fixed client logs "Submitting merkle batch payment on-chain (depth=N)" at info level | |
| # before calling payForMerkleTree. Confirms we did not hit some other code path | |
| # (e.g. a silent Auto-mode fallback to per-chunk `payForQuotes`). | |
| if grep -q "Submitting merkle batch payment on-chain" "${MERKLE_LOG}" 2>/dev/null; then | |
| MERKLE_MARKER_FOUND=true | |
| MERKLE_DEPTH=$(grep -oE "depth=[0-9]+" "${MERKLE_LOG}" | head -1 | cut -d= -f2) | |
| else | |
| fail "Merkle batch upload" "Upload reported success but merkle log line not found in stderr — possible silent fallback" | |
| echo " Stderr tail:" | |
| tail -10 "${MERKLE_LOG}" 2>/dev/null || true | |
| fi | |
| if [ "${UPLOAD_MARKER_FOUND}" = true ] && [ "${MERKLE_MARKER_FOUND}" = true ]; then | |
| pass "Merkle batch upload (${MERKLE_CHUNKS:-?} chunks, depth=${MERKLE_DEPTH:-?})" |
There was a problem hiding this comment.
Good catch, addressed in 266a190. Added an explicit UPLOAD_OK flag tracking the upload exit status, then check each marker (stdout "Upload complete", stderr merkle log line) independently and fail for each missing marker rather than treating absence as skip. Verified against three scenarios: happy path → 2 PASS, exit-0-without-markers → 2 FAIL, nonzero exit → 1 FAIL with no downstream runs.
| MERKLE_CHUNKS=$(grep "Chunks:" "${CLI_STDOUT}" | head -1 | grep -oE '[0-9]+' | head -1) | ||
| # The fixed client logs "Submitting merkle batch payment on-chain (depth=N)" at info level | ||
| # before calling payForMerkleTree. Confirms we did not hit some other code path | ||
| # (e.g. a silent Auto-mode fallback to per-chunk `payForQuotes`). | ||
| if grep -q "Submitting merkle batch payment on-chain" "${MERKLE_LOG}" 2>/dev/null; then | ||
| MERKLE_DEPTH=$(grep -oE "depth=[0-9]+" "${MERKLE_LOG}" | head -1 | cut -d= -f2) |
There was a problem hiding this comment.
With set -euo pipefail, the MERKLE_CHUNKS=$(grep ... | ...) pipeline will terminate the script if any grep in the pipeline finds no match (exit 1). Since chunk output format isn’t guaranteed, guard these extractions (e.g., tolerate non-matches) so the script records a clean FAIL instead of exiting early.
| MERKLE_CHUNKS=$(grep "Chunks:" "${CLI_STDOUT}" | head -1 | grep -oE '[0-9]+' | head -1) | |
| # The fixed client logs "Submitting merkle batch payment on-chain (depth=N)" at info level | |
| # before calling payForMerkleTree. Confirms we did not hit some other code path | |
| # (e.g. a silent Auto-mode fallback to per-chunk `payForQuotes`). | |
| if grep -q "Submitting merkle batch payment on-chain" "${MERKLE_LOG}" 2>/dev/null; then | |
| MERKLE_DEPTH=$(grep -oE "depth=[0-9]+" "${MERKLE_LOG}" | head -1 | cut -d= -f2) | |
| MERKLE_CHUNKS=$(grep "Chunks:" "${CLI_STDOUT}" 2>/dev/null | head -1 | grep -oE '[0-9]+' | head -1 || true) | |
| # The fixed client logs "Submitting merkle batch payment on-chain (depth=N)" at info level | |
| # before calling payForMerkleTree. Confirms we did not hit some other code path | |
| # (e.g. a silent Auto-mode fallback to per-chunk `payForQuotes`). | |
| if grep -q "Submitting merkle batch payment on-chain" "${MERKLE_LOG}" 2>/dev/null; then | |
| MERKLE_DEPTH=$(grep -oE "depth=[0-9]+" "${MERKLE_LOG}" 2>/dev/null | head -1 | cut -d= -f2 || true) |
| fi | ||
| fi |
There was a problem hiding this comment.
Round-trip validation is currently optional: if the expected datamap isn’t created at ${MERKLE_FILE%.bin}.datamap, the step just skips download/hash comparison without recording a failure. Since the goal is to loudly catch merkle-path regressions, treat missing datamap (and missing downloaded file) as a FAIL so this step can’t silently pass.
| fi | |
| fi | |
| fi | |
| else | |
| fail "Merkle batch download" "Download command completed but output file was not created at ${MERKLE_DOWNLOAD}" | |
| fi | |
| else | |
| fail "Merkle batch round-trip" "Expected datamap not created at ${DATAMAP_PATH}" |
Address Copilot review on PR WithAutonomi#82: the original Step 5b gated all downstream subtests on a stdout substring, so an upload that exited 0 without the expected "Upload complete" marker would silently pass without recording anything in the test counters. This restructures the step around an explicit `UPLOAD_OK` flag: - Track the upload command's exit status directly into UPLOAD_OK. - Check both markers (stdout "Upload complete", stderr merkle log line) independently and `fail` for each missing marker rather than treating their absence as "skip". - Gate the round-trip download on UPLOAD_OK rather than on the marker grep, and report concrete failures when the datamap is missing or the downloaded file fails to materialise. Manually exercised against a 16-node Anvil-backed devnet: Happy path → 2 PASS (upload + round-trip) Exit 0, no markers → 2 FAIL (one per missing marker, no silent skip) Exit nonzero → 1 FAIL, no follow-up subtests run
Summary
The existing e2e script never exercised
payForMerkleTree:CANDIDATES_PER_POOL = 16, sopay_for_merkle_batchreturnedInsufficientPeersandAuto-mode silently fell back to per-chunkpayForQuotes(which has no pool-count validation).ugly_files/are capped at 1 MB, well below the 64-chunk merkle threshold.--merkle, so even with bigger files Auto fallback would mask any contract-side failure.Net effect: zero coverage for the merkle path. The
WrongPoolCount(16, 8)production revert against odd-depth merkle trees (WithAutonomi/evmlib#9) would not have been caught by this script.Changes
DEVNET_NODESdefault bumped from 5 to 16 (+BOOTSTRAP_COUNT2 → 3), with a comment explaining the threshold. Existing override viaANT_TEST_DEVNET_NODESstill works.--merkleto disable Auto fallback,Submitting merkle batch payment on-chainlog line is present, so a future silent-fallback regression fails the test loudly,Override file size with
ANT_TEST_MERKLE_FILE_MB; skip the step entirely withANT_TEST_SKIP_MERKLE=1(for disk-constrained CI).Test plan
PASS: Merkle batch upload (74 chunks, depth=7)PASS: Merkle batch round-trip (SHA256 match)ANT_TEST_SKIP_MERKLE=1correctly skips the step.bash -n scripts/test_e2e.shclean.Out of scope
The other steps in
test_e2e.shuse stale CLI flags (--timeout-secs,--log-level,--output) that don't exist on the currentantbinary. Those failures are pre-existing and unrelated to this PR. Step 5b uses the current flag names (--quote-timeout-secs,--store-timeout-secs,-v,-o) so it works in isolation; happy to follow up with a separate PR fixing the others if useful.