Fix/ci test run - #42
Open
F-OBrien wants to merge 7 commits into
Open
Conversation
Vault crash-looped on GitHub Actions with "Failed to lock memory: cannot allocate memory", so vault-init sat in its readiness loop until the 120s timeout and exited 1, failing the whole job. The image runs as non-root (uid 100) and /bin/vault carries no cap_ipc_lock file capability, so `cap_add: IPC_LOCK` never reached the process's effective set and mlock stayed bound by RLIMIT_MEMLOCK. That limit is unlimited in containers on a typical workstation but finite on the runners, which is why this only ever reproduced in CI. Verified by reproducing the crash locally with --ulimit memlock=67108864. It is not version specific: hashicorp/vault:1.20.4 has no file capability either, so pinning VAULT_VERSION would not have helped. Drop the inert cap_add, disable mlock in the Vault config, and document the service as development only, since a reader copying it would inherit no TLS, a single unseal key, and a plaintext root token on a shared volume.
`start-env.sh` runs `docker compose up --detach`, which never attaches to container stdout. A failing one-shot service such as vault-init therefore reported only "didn't complete successfully: exit 1" with no indication of the cause, both locally and in CI. - start-env.sh: dump `ps --all` and `logs` when `compose up` fails - vault-init.sh: check READY before reading the token file, so a timeout no longer emits an empty "Vault Root Token:" line and a stray cat error, and no longer marks the service available on a failed run. Report the last Vault status seen along with the timeout - vault-init.sh: report the out-of-sync volume case in unseal_vault rather than aborting on `cat` of a missing .unseal_key
The job hit the 20 minute ceiling. Because that is a job-level timeout, GitHub cancels the job rather than failing it, so the `if: failure()` diagnostics step never ran and the run reported no logs at all. Put the real limits on the individual steps and keep the job ceiling above their sum, so an overrunning phase trips its own timeout while the job still has budget to dump logs. Splitting `yarn test` into `test:start` and `test:run` also reports environment startup time separately from the suite itself, which the combined step hid. Teardown moves to an `if: always()` step, since test.sh uses `set -e` and skipped it whenever anything failed. The limits are sized from a measured run: 5m22s to start the environment against 12 minutes, and 27m45s for the suite against 40. Startup includes pulling every image, since the runners keep no layer cache between runs. Bump checkout and setup-node to v5 to clear the Node 20 deprecation warning.
@polkadot/util, util-crypto and the wasm-crypto packages were each being installed twice, at 13.5.8 and 13.5.9, one hoisted and one under the SDK. Pin them to the versions the SDK resolves so only one copy is installed. Note this does not silence the "@polkadot/x has multiple versions" banners in the test output. Those are the same version registering itself twice through its ESM and CJS entry points under jest, which @polkadot/util reports through the same warning.
34 of 56 suites failed with "createTestAccounts failed (422): Unprocessable: Insufficient free balance" and "does not have enough POLYX balance to pay this transaction's fees". Each Jest worker has its own admin, and that admin paid 100_000 POLYX for every identity its suites create. The suite creates about 80 identities across 8 workers, so demand was 8_000_000 against a supply of 8 x 1_000_000: exactly 100% subscribed, with nothing left for fees. An admin could afford 10 identities and needed 10 on average, so any worker handed an above-average share of suites ran dry. That is why roughly half the suites failed rather than all or none. The admin float cannot absorb it. createTestAdmins transfers from a finite source account and fails on chain with FundsUnavailable when asked for more than it holds: probing a local environment showed 8 x 1_000_000 succeeding while both 8 x 2_500_000 and 8 x 10_000_000 failed outright, leaving every admin empty. Amount alone does not decide it either, identical requests start failing once the source is drawn down. So take the headroom from the other side. 25_000 per identity cuts demand to 2_000_000 and leaves 4x headroom per worker, while still dwarfing what a suite spends: protocol fees are 2_500 to create an asset and 500 for a ticker, and no test moves more than 1_000 POLYX. Check the createTestAdmins response as well, so a funding failure fails the run at its cause instead of surfacing as hundreds of unrelated test failures.
…affirmation The test asserted "the counter party should have the instruction as pending" and timed out. Probing a local v8 environment shows two reasons it can never be there, and that the assertion was asking for the wrong thing in the first place. Receiver affirmation is automatic from v8 unless the receiver opts in through settlement.setMandatoryReceiverAffirmation. The counter party only receives on this single leg instruction, so it is affirmed at submission and the instruction appears in its `affirmed` list, never in `pending`. The sibling in sdk/settlements/tradeAssets.ts still passes because its counter party sends on the second leg, and a sender must always affirm. The instruction also carried an `endBlock` 50 blocks out, added when the suite moved to 8.x to stop instructions auto-settling. That makes it SettleOnBlock, so it stays Pending until that block regardless of affirmations, and the controller transfer that follows needs the counter party to actually hold the asset. Waiting for a pending affirmation could not have produced that; only settlement can. Drop the end block and wait for the instruction to execute, affirming as the receiver only when the chain requires it, which keeps the example working on 7.x. Reading the instruction's own chain state also removes the dependency on the counter party's middleware-backed instruction list, which lags the chain under load. While here, the holder assertions were `expect(x.balance.eq(y))` with no matcher, so they asserted nothing and let the unsettled transfer through. Compare the balances properly and wait for the middleware to catch up with the controller transfers before reading holders.
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
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.
Summary
The
Run Testsworkflow could not complete. Vault crash-looped on the GitHub runners so the environment never came up, and once that was fixed a series of further failures surfaced behind it — admin accounts running out of POLYX, a job timeout that cancelled the run before diagnostics could print, and a settlement test asserting on state that can no longer occur on chain v8.This gets the workflow running end to end and fixes the failures it exposed. Also adds a small unrelated quality-of-life change: disabling the third-party ad slots the upstream Blockscout frontend ships with.
Changes
Vault would not start on CI runners
cap_add: IPC_LOCKwas inert — the image runs as non-root (uid 100) and/bin/vaultcarries nocap_ipc_lockfile capability, so the capability never reached the process's effective set andmlockstayed bound byRLIMIT_MEMLOCK. That limit is unlimited in containers on a typical workstation but finite on the runners, which is why this only ever reproduced in CI. Reproduced locally with--ulimit memlock=67108864to confirm.Drops the inert
cap_addand setsdisable_mlockin the Vault config. Not version specific —hashicorp/vault:1.20.4has no file capability either, so pinningVAULT_VERSIONwould not have helped.The Vault service is now documented as development only, since a reader copying it would otherwise inherit no TLS, a single unseal key, and a plaintext root token on a shared volume.
Failures reported no cause
start-env.shrunsdocker compose up --detach, which never attaches to container stdout, so a failing one-shot service such asvault-initreported onlydidn't complete successfully: exit 1. It now dumpsps --allandlogson failure, both locally and in CI.vault-init.shalso checks readiness before reading the token file, so a timeout no longer emits an emptyVault Root Token:line and a straycaterror, and no longer marks the service available on a failed run.CI job structure
The job hit its 20 minute ceiling. Because that is a job-level timeout, GitHub cancels the job rather than failing it, so the
if: failure()diagnostics step never ran and the run reported no logs at all.yarn testis split intotest:startandtest:runwith their own step timeouts, sized from a measured run (5m22s startup against 12 minutes, 27m45s suite against 40). The job ceiling sits above their sum so an overrunning phase trips its own step timeout while the job still has budget to dump logs. Teardown moves to anif: always()step, sincetest.shusesset -eand skipped it whenever anything failed.checkoutandsetup-nodebumped to v5 to clear the Node 20 deprecation warning.Test admins ran out of POLYX
34 of 56 suites failed with
Insufficient free balanceanddoes not have enough POLYX balance to pay this transaction's fees.Each Jest worker has its own admin, and that admin paid 100,000 POLYX for every identity its suites create. The suite creates ~80 identities across 8 workers, so demand was 8,000,000 against a supply of 8 × 1,000,000 — exactly 100% subscribed, with nothing left for fees. An admin could afford 10 identities and needed 10 on average, so any worker handed an above-average share of suites ran dry, which is why roughly half the suites failed rather than all or none.
The admin float cannot absorb it:
createTestAdminstransfers from a finite source account and fails on chain withFundsUnavailablewhen asked for more than it holds. Probing a local environment showed 8 × 1,000,000 succeeding while both 8 × 2,500,000 and 8 × 10,000,000 failed outright, leaving every admin empty.So the headroom comes from the other side: 25,000 POLYX per identity cuts demand to 2,000,000 and leaves 4x headroom per worker, while still dwarfing what a suite spends (protocol fees are 2,500 to create an asset and 500 for a ticker, and no test moves more than 1,000 POLYX). The
createTestAdminsresponse is now checked too, so a funding failure fails the run at its cause instead of as hundreds of unrelated test failures.controllerTransferasserted on impossible stateThe test timed out on
the counter party should have the instruction as pending. Probing a local v8 environment showed two reasons it can never be there, and that the assertion was asking for the wrong thing.Receiver affirmation is automatic from v8 unless the receiver opts in through
settlement.setMandatoryReceiverAffirmation. The counter party only receives on this single-leg instruction, so it is affirmed at submission and lands in itsaffirmedlist, never inpending. (The sibling insdk/settlements/tradeAssets.tsstill passes because its counter party sends on the second leg, and a sender must always affirm.)The instruction also carried an
endBlock50 blocks out, added when the suite moved to 8.x to stop instructions auto-settling. That makes itSettleOnBlock, so it staysPendinguntil that block regardless of affirmations — and the controller transfer that follows needs the counter party to actually hold the asset. Waiting for a pending affirmation could never produce that; only settlement can.Now drops the end block and waits for the instruction to execute, affirming as the receiver only when the chain requires it (which keeps the example working on 7.x). Reading the instruction's own chain state also removes the dependency on the counter party's middleware-backed instruction list, which lags the chain under load.
While in there: the holder checks were
expect(x.balance.eq(y))with no matcher, so they asserted nothing and let an unsettled transfer through. They now compare balances properly, and the middleware is given a chance to catch up with the controller transfers before holders are read.Dependencies
@polkadot/util,util-cryptoand thewasm-cryptopackages were each being installed twice, at 13.5.8 and 13.5.9, one hoisted and one under the SDK. Now pinned to the versions the SDK resolves. SDK bumped to 30.1.0.Note this does not silence the
@polkadot/x has multiple versionsbanners in the test output — those are the same version registering itself twice through its ESM and CJS entry points under Jest, reported through the same warning.Blockscout ads
The upstream frontend image enables third-party ad slots by default (a banner on the home page and a text ad on token pages). Disabled here, with
POLYMESH_BLOCKSCOUT_AD_BANNER_PROVIDER/..._AD_TEXT_PROVIDERto re-enable.Verification
controllerTransferpasses locally against a fresh dev environment (✓ should execute controllerTransfer without errors), run twice.createTestAdminsfunding ceiling probed directly against a local environment.controllerTransferthe only genuine remaining failure — that is the fix in this PR, so a green run is expected but has not been observed yet.Notes for reviewers
nonFungibleAssetControllerTransferis unused but received the same fix, to keep the two examples consistent.