Skip to content

test(e2e): cover the merkle batch payment path#82

Merged
mickvandijke merged 2 commits intoWithAutonomi:mainfrom
grumbach:feat/e2e-merkle-coverage
Apr 27, 2026
Merged

test(e2e): cover the merkle batch payment path#82
mickvandijke merged 2 commits intoWithAutonomi:mainfrom
grumbach:feat/e2e-merkle-coverage

Conversation

@grumbach
Copy link
Copy Markdown
Collaborator

Summary

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 WrongPoolCount(16, 8) production revert against odd-depth merkle trees (WithAutonomi/evmlib#9) would not have been caught by this script.

Changes

  • DEVNET_NODES default bumped from 5 to 16 (+ BOOTSTRAP_COUNT 2 → 3), with a comment explaining the threshold. Existing 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).

Test plan

  • Verified locally against a 16-node Anvil-backed devnet (release build):
    • PASS: Merkle batch upload (74 chunks, depth=7)
    • PASS: Merkle batch round-trip (SHA256 match)
  • Verified ANT_TEST_SKIP_MERKLE=1 correctly skips the step.
  • bash -n scripts/test_e2e.sh clean.

Out of scope

The other steps in test_e2e.sh use stale CLI flags (--timeout-secs, --log-level, --output) that don't exist on the current ant binary. 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.

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)
Copilot AI review requested due to automatic review settings April 27, 2026 04:03
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

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.

Comment thread scripts/test_e2e.sh Outdated
Comment on lines +391 to +404
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

Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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:-?})"

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

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.

Comment thread scripts/test_e2e.sh Outdated
Comment on lines +392 to +397
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)
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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)

Copilot uses AI. Check for mistakes.
Comment thread scripts/test_e2e.sh
Comment on lines +427 to +428
fi
fi
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

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.

Suggested change
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}"

Copilot uses AI. Check for mistakes.
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
@mickvandijke mickvandijke merged commit 23aee15 into WithAutonomi:main Apr 27, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants