try to fix dev timeout#2765
Conversation
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE BASELINE scrutiny: open-junius has write permission, >1 year old account, substantial prior contribution history, and no Gittensor allowlist match found; branch fix-dev-timeout -> devnet-ready. The diff does not touch runtime/pallet code, Cargo dependencies, workflows, trusted AI-review files, or Findings
Prior-comment reconciliation
ConclusionBlock until the lockfile no longer resolves 📜 Previous run (superseded)
🔍 AI Review — Auditor (domain review)VERDICT: 👎 LIKELY gittensor / established contributor: no allowlist match, but open-junius has repo write permission and substantial recent subtensor PR activity. The Auditor proposed a replacement PR description, but the current body is non-trivial; not overwriting. Maintainers: ask the Auditor to regenerate if you want it. No Auto-fix: not needed. I did not run the Moonwall E2E suite because the concurrency issue is visible from the config and existing dev tests. Duplicate-work check: PRs #2728, #2716, and #2715 overlap on Findings
ConclusionBlock for now. The only behavioral change enables concurrent execution for stateful dev E2E suites that share accounts and chain state, which is likely to make the timeout/flakiness problem worse rather than safer. |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ], | ||
| "runScripts": [], | ||
| "multiThreads": false, | ||
| "multiThreads": true, |
There was a problem hiding this comment.
[MEDIUM] Do not parallelize the shared dev chain tests
The dev Moonwall suite is not isolated per test file: the existing files under suites/dev reuse shared keyring accounts such as Alice/Bob, mutate global chain state, register subnets, sudo-set parameters, and manually create blocks against the configured dev foundation. Enabling multiThreads lets those files run concurrently against that same environment, so nonce reads, block creation, subnet IDs, balances, and pallet state can race each other. That makes the timeout fix non-deterministic and can turn passing tests into flakes.
Keep this disabled unless the suite is first split so each worker gets an isolated node/state snapshot or the tests are made concurrency-safe.
| "multiThreads": true, | |
| "multiThreads": false, |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
| toml@https://codeload.github.com/pepoviola/toml-node/tar.gz/5e17114f1af5b5b70e4f2ec10cd007623c928988: | ||
| resolution: {gitHosted: true, tarball: https://codeload.github.com/pepoviola/toml-node/tar.gz/5e17114f1af5b5b70e4f2ec10cd007623c928988} | ||
| version: 3.0.0 |
There was a problem hiding this comment.
[HIGH] Unvetted GitHub tarball dependency introduced in lockfile
The PR removes the toml: 3.0.0 override and changes @moonwall/* resolution from the npm toml@3.0.0 package with integrity metadata to a GitHub codeload tarball (pepoviola/toml-node at a commit). That is a new git-hosted supply-chain input for CI/test installs, unrelated to the stated timeout/config change, and the lockfile entry has no registry integrity hash. Restore the override/regenerate the lockfile so toml resolves to the npm package, or provide an explicit security review/justification for this source change.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| toml@https://codeload.github.com/pepoviola/toml-node/tar.gz/5e17114f1af5b5b70e4f2ec10cd007623c928988: | ||
| resolution: {tarball: https://codeload.github.com/pepoviola/toml-node/tar.gz/5e17114f1af5b5b70e4f2ec10cd007623c928988} | ||
| version: 3.0.0 |
There was a problem hiding this comment.
[HIGH] Unvetted GitHub tarball dependency remains in lockfile
This adds a reachable toml package resolution from codeload.github.com; the snapshots later resolve @zombienet/utils to this tarball instead of the npm registry toml@3.0.0. GitHub tarball dependencies bypass the normal registry integrity/provenance path and would be fetched when CI installs the test dependencies. Restore the workspace/lockfile override that pins toml to the registry package, or provide explicit vetting and justification for this source before merging.
|
🔄 AI review updated — Skeptic: VULNERABLE |
1 participant
Description
Noticed the dev test timeout in ts-tests, check if it is from multithread parameter.
Related Issue(s)
Type of Change
Breaking Change
If this PR introduces a breaking change, please provide a detailed description of the impact and the migration path for existing applications.
Checklist
./scripts/fix_rust.shto ensure my code is formatted and linted correctlyScreenshots (if applicable)
Please include any relevant screenshots or GIFs that demonstrate the changes made.
Additional Notes
Please provide any additional information or context that may be helpful for reviewers.