fix(contracts,chain): V10 dual-emit + ABI regen + hardhat test rewrites (1/4)#331
fix(contracts,chain): V10 dual-emit + ABI regen + hardhat test rewrites (1/4)#331
Conversation
Fixes the on-chain layer of the bug-fix backlog from PR #327. This is slice 1 of 4 (split by architectural layer for reviewability). Bug fixes - E-2/E-14/E-16: Hub access-control hardening on owner-only paths. - E-6: DKGPublishingConvictionNFT now reverts AccountExpired guard before any state mutation (defence-in-depth around the lock-tier ladder). - CH-5: KnowledgeAssetsV10 / KnowledgeAssetsStorage now dual-emit V10KnowledgeBatchEmitted so off-chain consumers can filter on a single ABI'd event regardless of which contract minted the batch. - MigratorV10Staking: backfill helper for the V10 staking aggregates (totalStakeV10, getNodeStakeV10) used by the migrator path. ABI regen - packages/chain/abi/{KnowledgeAssetsStorage,KnowledgeAssetsV10, DKGStakingConvictionNFT}.json — synced so ethers.js Contract.filters exposes V10KnowledgeBatchEmitted (was missing → evm-e2e.test.ts TypeError on main). - packages/evm-module/abi/{KnowledgeAssetsStorage,KnowledgeAssetsV10, MigratorV10Staking}.json — same regen against the canonical source. Hardhat tests rewritten against actual V10 API The three files below were authored against a pre-Phase-5 NFT API that never shipped (NFT.stake / partial-withdraw / stakingStorageAddress). Each was rewritten to use createConviction / withdraw (full-only) / stakingStorage public state var, with assertions reading Position records directly from ConvictionStakingStorage: - DKGStakingConvictionNFT-extra.test.ts — full V10 NFT lifecycle. - v10-conviction-extra.test.ts — lock-tier multiplier ladder. The lock=1 expectation was corrected from 1.0x → 1.5x to match the baseline tier table seeded by ConvictionStakingStorage._seedBaselineTiers. - v10-conviction-nft-audit.test.ts — refocused on the E-6 AccountExpired guard. Duplicated E-2/E-14/E-16 cases were removed (covered in v10-hub-audit / v10-kav10-audit). EVM-side tests - packages/chain/test/evm-e2e.test.ts: now compiles after the ABI regen exposes the V10KnowledgeBatchEmitted filter. - mock-adapter-behavioral.test.ts: new behavioural coverage for the off-chain mock used in agent/publisher tests. CI / infra (drive-by, all on this layer's review surface) - codex-review.yml: timeout-minutes 15 → 30 (the bot was hitting the cap on >30k-line PRs; concurrency-cancel still kicks in on new pushes). - release.yml + npm-continuous-publish.yml: comment-only formatting. Coverage ratchet - tornadoChainCoverage thresholds bumped to reflect the new mock-adapter-behavioral coverage (24/28/14/23 → 73/80/58/72). Sequencing: this is PR 1 of 4. Independent off main; PRs 2-4 carry the storage+query, operator (cli+mcp+openclaw), and pipeline (publisher+agent+network-sim) layers respectively. Original umbrella branch parked at refs/backup/fix_tests-pre-split-* and on fix_tests-overflow (adapter-elizaos + node-ui drive-bys). Made-with: Cursor
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| } | ||
|
|
||
| const tx = await nft.stake(identityId, amount, lockEpochs); | ||
| const tx = await nft.createConviction(identityId, amount, lockEpochs); |
There was a problem hiding this comment.
🔴 Bug: stakeWithLock still exposes the third argument as lockEpochs, but createConviction now interprets that value as a discrete lockTier (0/1/3/6/12). Callers that previously passed epoch counts like 2 will now revert with InvalidLockTier, while the mock adapter still treats 2 as valid and returns a 1.5x multiplier. Either translate epoch counts to tiers here, or rename/update the chain-adapter API and mock implementation so both adapters agree on the new contract semantics.
| // so we cannot reconstruct the on-chain values without a wall | ||
| // clock — emit schema-compatible zeros and leave epoch-window | ||
| // assertions to the EVM e2e suite. | ||
| this.pushEvent('V10KnowledgeBatchEmitted', { |
There was a problem hiding this comment.
🔴 Bug: This is the legacy publishKnowledgeAssets path, but it now emits V10KnowledgeBatchEmitted, which the real EVM adapter only exposes for V10/KAV10 publishes. That gives mock-backed tests false positives for V10 listeners, and the actual mock V10 path (createKnowledgeAssetsV10) still does not emit this event at all. Keep legacy publishes on KnowledgeBatchCreated/KCCreated, and move the V10 audit event emission to the real V10 publish method.
| ).to.equal(true); | ||
| }); | ||
|
|
||
| it('SPEC-GAP: MigratorV10Staking compiled artifact must resolve', () => { |
There was a problem hiding this comment.
🔴 Bug: The new migrator writes to DelegatorsInfo, per-node stake, and global totalStake, but this suite only checks file/artifact/ABI presence. That means regressions in migrateDelegator/markNodeMigrated can still ship undetected. Please add a fixture-based test that deploys the contract, registers it in Hub, and verifies happy-path state changes plus the duplicate/unknown-id/finalize guards against real storage state.
1 participant
Summary
PR 1 of 4 in the split of #327. Each slice is a single architectural layer to keep reviews tractable.
This PR carries the on-chain layer: Solidity bug fixes, ABI regeneration, and Hardhat test rewrites.
Changes by category
Smart-contract bug fixes
DKGPublishingConvictionNFTnow reverts on `AccountExpired` before any state mutation.ABI regeneration
Hardhat test rewrites (against actual V10 API)
The three files below were authored against a pre-Phase-5 NFT API that never shipped (`NFT.stake` / partial-withdraw / `stakingStorageAddress`). Rewritten to use `createConviction` / `withdraw` (full-only) / `stakingStorage` public state var, with assertions reading `Position` records directly from `ConvictionStakingStorage`:
EVM-side tests
CI / infra (drive-by, on this layer's review surface)
Coverage ratchet
Sequencing context
This is slice 1 of 4 from the #327 split (by architectural layer):
`adapter-elizaos` overhaul (~8.5k) and `node-ui` chat-memory overhaul (~2.2k) parked on `fix_tests-overflow` for separate review — neither was failing on `main`'s CI.
Original umbrella branch backed up at `refs/backup/fix_tests-pre-split-1777483348`.
Test plan
Made with Cursor