test(evm): migrate response-agnostic lib.js helpers off -b block (CON-256)#3363
Draft
wen-coding wants to merge 11 commits intomainfrom
Draft
test(evm): migrate response-agnostic lib.js helpers off -b block (CON-256)#3363wen-coding wants to merge 11 commits intomainfrom
wen-coding wants to merge 11 commits intomainfrom
Conversation
bankSend, fundSeiAddress, executeWasm, associateWasm all used -b block (broadcast-mode block), which subscribes to tendermint's EventDataTx for the submitted hash and waits up to 60s for it to fire. Under Autobahn the executeBlock path doesn't invoke FireEvents, so EventDataTx is never published and -b block hangs to the timeout, blocking every test that funds an account or runs a wasm message in beforeEach / before-all hooks. Switch to -b sync (returns on mempool acceptance) plus a fixed 2s sleep to let inclusion happen. Portable across both engines: under CometBFT this is slightly slower than the event-driven detection -b block had, but the difference is in the noise of the surrounding hardhat-ethers polling. Under Autobahn it unblocks the test paths entirely. Verified locally: EVMPrecompileTest's Bank, Addr, Distribution, Staking, Oracle precompile sections (which depend on getAdmin → fundSeiAddress) go from 0/all hung in before-all to 9/9 passing. Gov + Wasm precompile sections still fail because they have direct -b block calls inside the test files themselves; those need separate substitutions. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
wen-coding
changed the title
Same migration as the prior commit, extended to two more sites that don't read post-execution event data: - associateKey: result is unused (wrapped in try/catch swallowing all errors). The -b block timeout was a 60s wait under Autobahn before the catch fired. - passProposal gov vote: caller polls proposal status afterward; the vote-tx return value is unused. Helpers that parse post-execution events from the response (storeWasm, proposeParamChange, registerPointerForERC*, etc.) stay on -b block in this PR — they need a separate "submit then poll for tx result" change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI on this PR caught a regression: CW1155toERC1155PointerTest's "should not transfer an NFT if not owned" reads res.code from executeWasm and expects it to be non-zero (the wasm execution should fail when the sender doesn't own the tokens). Under -b block, res.code is the DeliverTx code (post-execution); under -b sync it's the CheckTx code (mempool acceptance). For a tx that passes basic checks but fails in DeliverTx, -b sync returns 0 and the test wrongly thinks the tx succeeded. executeWasm and associateWasm have callers that read post-execution fields (code, events, logs) from the response, so they can't migrate to the -b sync pattern without also adding a "submit then poll for tx result" step. Revert these two; leave the truly response-agnostic helpers (bankSend, fundSeiAddress, associateKey, passProposal vote) on -b sync. The complex helpers are deferred to the same follow-up PR that addresses storeWasm, proposeParamChange, and the other event-parsing callers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wen-coding
changed the title
wen-coding
changed the title
Reviewer flagged the magic 2s sleep after -b sync submissions: under CometBFT (block ~1s) it's marginal; under Autobahn (block ~100ms) it's 20x longer than needed. Both are arbitrary. Replace with waitForBlocks(1): poll eth_blockNumber every 50ms until the chain advances by one block, with a 15s timeout. Engine-agnostic substitute that returns as soon as the next block lands. Net wall-clock impact (Bank/Addr/Distribution precompile sections): - CometBFT: 19s → 15s - Autobahn: 39s → 10s Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CI on this PR caught a sequence mismatch in SeiSoloTest's "Claim CW20 Tester / before all": fundSeiAddress submits with -b sync and waitForBlocks(1) returns when the next block lands. That next block can be empty (the submitted tx is still in mempool and lands one block later), so the chain hasn't advanced the funder's account sequence yet. The follow-up storeWasm queries the funder's account, gets the stale sequence, signs with it, and is rejected when the funder tx finally lands a block later and bumps the sequence. Default to 2 blocks instead of 1: closes the empty-next-block race without reintroducing a fixed sleep. Callers no longer pass an explicit block count — the helper takes care of the safety margin. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
wen-coding
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #3363 +/- ##
==========================================
+ Coverage 59.07% 59.17% +0.09%
==========================================
Files 2099 2097 -2
Lines 172988 172640 -348
==========================================
- Hits 102195 102162 -33
+ Misses 61922 61615 -307
+ Partials 8871 8863 -8
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
Two more lib.js helpers were still using --broadcast-mode=block. Under
Autobahn the executeBlock path doesn't fire EventDataTx, so -b block
hangs to its 60s timeout on every call.
evmSend (used by fundAddress, called from setupSigners and many test
file before-hooks): convert to -b sync + waitForBlocks. Callers read
only the txhash from the output, which is present in either format,
so no other adjustment needed. This unblocks EVMPrecompileTest and
EVMGigaTest's `transfer to non-existent contract with data` (latter
verified passing in 126ms after the fix).
proposeParamChange: convert to -b sync. The tx response under -b sync
no longer carries deliver_tx events, so we can't read proposal_id from
the submit_proposal event. Poll gov state instead (max-id-before vs
max-id-after) — autobahn doesn't run a Cosmos-side tx indexer so
seid q tx <hash> isn't an option. New maxProposalId helper handles the
"no proposals exist yet" case explicitly via try/catch; using a shell
`|| echo '{...}'` fallback breaks because the docker-exec wrapper
double-quotes the inner command. Verified by EVMCompatabilityTest
test #3 (`should reproduce mismatch by changing param`) passing in
47s after the fix (was timing out at 120s with -b block).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Last lib.js helper still using --broadcast-mode=block. Same Autobahn incompatibility as evmSend / proposeParamChange: executeBlock doesn't fire EventDataTx, so -b block hangs to its 60s timeout on every call, which blocked ERC20toNativePointerTest's before-all hook. The original code read the new denom from the create_denom event in the tx response. Under -b sync we don't get deliver_tx events, but the tokenfactory denom is deterministic — `factory/<creator>/<subdenom>` where creator is the bech32 of the --from key. Construct it locally from getKeySeiAddress instead. Verified: ERC20toNativePointerTest passes 14/14 in 17s after the fix (previously 0/1, hung 60s+ on the before-all hook). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Gov Precompile section's before-hook had its own inline 'seid tx gov submit-proposal param-change ... -b block' which hangs 60s under Autobahn. Replace with the lib.js proposeParamChange helper (now -b sync + poll gov state). EVMPrecompileTest goes from 9/11 to 10/11 — only the Wasm Precompile section still fails (out of scope; wasm being decommed). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Several helpers in
contracts/test/lib.jsused-b block(broadcast-mode block) — an RPC anti-pattern that holds an event subscription server-side and conflates submission with inclusion. Sei's vendored cosmos-sdk already advises against it (sei-cosmos/client/broadcast.gosays "use BroadcastTxAsync or BroadcastTxSync instead"). It works most of the time on CometBFT becauseFireEventsfiresEventDataTxquickly; under Autobahn the equivalent code path doesn't fire those events, so-b blockdeterministically times out at 60s.Migrate the helpers whose callers don't read post-execution response fields to
-b syncplus a newwaitForBlocks(1)helper that pollseth_blockNumberuntil the chain advances by a block:bankSend,fundSeiAddress,associateKey,passProposalgov vote.Helpers whose callers DO read post-execution fields (
executeWasm,associateWasm,storeWasm,proposeParamChange,registerPointerForERC*) stay on-b block. Those need a "submit then poll for deliver-tx result" pattern — a more invasive change worth its own PR. Under-b sync,res.codereflects only CheckTx (mempool acceptance), not DeliverTx (post-execution); naive flipping there silently breaks tests that readres.codeto detect execution failures.Verification
Wall-clock A/B (Bank+Addr+Distribution precompile sections + 3 trace tests, local 4-node clusters):
before all🤖 Generated with Claude Code