Fix/deterministic cnight mock for multi node#1870
Conversation
Replace random UTXO generation with deterministic generation based on block number to ensure all nodes in a multi-node network produce identical inherent data. This prevents block verification failures due to inherent data mismatches. All nodes with the same block number now generate the same mock CNight observations (tx hashes, reward addresses, dust public keys), enabling multi-node development setups without Cardano infrastructure.
Signed-off-by: Lech Głowiak <LGLO@users.noreply.github.com>
|
Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits. |
Jina ReviewJina has completed this review. Review: https://app.usejina.com/reviews/6a3f4746-ff10-42b9-bd31-ee1d3190ed03
|
This comment has been minimized.
This comment has been minimized.
![jina-simulation[bot]](https://avatars.githubusercontent.com/u/97225700?s=60&v=4){kind=small}
There was a problem hiding this comment.
Runtime Review - Merge Readiness 2/5
3 issues found - Do not merge until addressed
Do not merge until addressed.
The accepted findings are confined to development mode, but invalid keys can break the intended CNight-to-Dust workflow immediately, while reward-address collisions become reachable during normal local operation and permanently make mappings ambiguous.
Persistent per-block state growth adds a substantial long-running cost.
Final review: Three grounded registration-state issues are accepted.
They affect development networks through address ambiguity, invalid Dust keys, and unbounded storage growth.
Review Comments
-
Boundary cursor handling is not covered
The mock increments block_number with unchecked u32 addition.
The investigation did not promote this to a publishable issue because the boundary is unlikely in ordinary development, but the position-progression area explicitly includes boundary behavior and the added tests do not cover u32::MAX.
A checked increment with an explicit data-source error would make this edge case safer.
Evidence:
- primitives/mainchain-follower/src/data_source/cnight_observation_mock.rs:119 uses end.block_number += 1.
- CardanoPosition.block_number is u32.
- Focused Cargo execution was unavailable.
Related files:
primitives/mainchain-follower/src/data_source/cnight_observation_mock.rs,pallets/cnight-observation/src/lib.rs
Issues
Inline comments posted
-
Deterministic reward addresses repeat every 256 blocks
Location:
primitives/mainchain-follower/src/data_source/cnight_observation_mock.rs:76; Risk / impact: high; Evidence confidence: high; Production likelihood: high; Category: data; Validation: hybridReward-address bytes are generated modulo 256, so blocks n and n+256 produce the same address. Distinct UTXO hashes create multiple Mapping entries for that address, causing the pallet to mark it ambiguous. This violates the area expectation that per-block mock identities avoid unintended registration ambiguity.
-
Generated Dust keys are not guaranteed to be valid public keys
Location:
primitives/mainchain-follower/src/data_source/cnight_observation_mock.rs:80; Risk / impact: high; Evidence confidence: medium; Production likelihood: high; Category: integration; Validation: hybridThe mock labels arbitrary 33-byte data with a 0x02 prefix as a compressed Dust public key. DustPublicKeyBytes validates only length. At genesis block 0 this produces 0x02 followed by 32 zero bytes, which downstream ledger deserialization rejects. This violates the area expectation that generated Dust keys satisfy downstream semantic requirements.
-
Every queried block permanently adds a Mapping row
Location:
primitives/mainchain-follower/src/data_source/cnight_observation_mock.rs:123; Risk / impact: medium; Evidence confidence: high; Production likelihood: high; Category: performance; Validation: hybridThe cadence change now returns one registration on every query, and process_tokens permanently inserts each registration into Mapping. This violates the area expectation that registration storage and event growth remain intentional and sustainable.
Summary
- Status: issues found
- Areas investigated: 4
- Tasks/probes performed: 3
- Issues reported: 3
- Publishable issues: 3
- Inline comments: 3
- File-level comments: 0
- Review comments: 1
- Unanchored publishable findings posted as file-level comments: 0
- Blocked validations: 1
- Dashboard: full runtime review
- Commit: d1bd4a8
- Changed files: 2
- Tool calls: none
| // Deterministic reward address (29 bytes) | ||
| let mut reward_addr = [0u8; 29]; | ||
| for (i, byte) in reward_addr.iter_mut().enumerate() { | ||
| *byte = ((block_num as u64 * (i as u64 + 7) * 17) % 256) as u8; |
There was a problem hiding this comment.
Deterministic reward addresses repeat every 256 blocks
Risk: high
Confidence: high
Likelihood: high
Grounding: runtime review, hybrid
What happens
Reward-address bytes are generated modulo 256, so blocks n and n+256 produce the same address. Distinct UTXO hashes create multiple Mapping entries for that address, causing the pallet to mark it ambiguous. This violates the area expectation that per-block mock identities avoid unintended registration ambiguity.
Root cause
The identity derivation has a 256-block period.
Why it matters
After approximately 25.6 minutes at six-second blocks, registrations become unusable because unique_dust_key returns None.
Evidence
- Related runtime area cnight_registration_state: Mock registration cadence and persistent CNight mapping state
- Area expectations: Emitting one registration every queried block has intentional, sustainable effects on Mapping storage and registration events.; Generated reward addresses and Dust keys satisfy downstream representation and semantic requirements.; Per-block mock identities vary sufficiently to avoid unintended registration ambiguity.; The mock remains within runtime UTXO capacity and weight assumptions.
- Reward bytes use ((block_num * (i + 7) * 17) % 256).
- Mapping is keyed by reward address and UTXO ID.
- unique_dust_key returns None for multiple mappings.
Reproduction or trace
Blocks 0 and 256 generate identical reward addresses but distinct UTXO hashes; the second registration transitions the address to an ambiguous state.
Suggested fix
Derive identities from a collision-resistant hash of the block position.
|
|
||
| let (dust_pk, _) = dust_pk.split_at((0..33).choose(&mut rng).unwrap()); | ||
| // Deterministic dust public key (33 bytes compressed) | ||
| let mut dust_pk = [0u8; 33]; |
There was a problem hiding this comment.
Generated Dust keys are not guaranteed to be valid public keys
Risk: high
Confidence: medium
Likelihood: high
Grounding: runtime review, hybrid
What happens
The mock labels arbitrary 33-byte data with a 0x02 prefix as a compressed Dust public key. DustPublicKeyBytes validates only length. At genesis block 0 this produces 0x02 followed by 32 zero bytes, which downstream ledger deserialization rejects. This violates the area expectation that generated Dust keys satisfy downstream semantic requirements.
Root cause
The mock fabricates serialized key bytes instead of deriving and serializing a valid DustPublicKey.
Why it matters
Registrations can be stored successfully while CNightGeneratesDustEvent construction fails, particularly from the genesis starting position.
Evidence
- Related runtime area cnight_registration_state: Mock registration cadence and persistent CNight mapping state
- Area expectations: Emitting one registration every queried block has intentional, sustainable effects on Mapping storage and registration events.; Generated reward addresses and Dust keys satisfy downstream representation and semantic requirements.; Per-block mock identities vary sufficiently to avoid unintended registration ambiguity.; The mock remains within runtime UTXO capacity and weight assumptions.
- DustPublicKeyBytes::try_from checks only the 33-byte bound.
- Block 0 produces [0x02, 0, ..., 0].
- construct_cnight_generates_dust_event deserializes the key.
- handle_create suppresses the event when construction fails.
Reproduction or trace
Register the block-0 mock UTXO, then process an asset create for its reward address; registration succeeds but ledger owner deserialization rejects the fabricated key.
Suggested fix
Derive a deterministic valid Dust key and serialize the resulting public key.
|
|
||
| let utxos = | ||
| if start.block_number.is_multiple_of(5) { mock_utxos(start) } else { Vec::new() }; | ||
| // Return deterministic UTXOs - same data on all nodes for the same block |
There was a problem hiding this comment.
Every queried block permanently adds a Mapping row
Risk: medium
Confidence: high
Likelihood: high
Grounding: runtime review, hybrid
What happens
The cadence change now returns one registration on every query, and process_tokens permanently inserts each registration into Mapping. This violates the area expectation that registration storage and event growth remain intentional and sustainable.
Root cause
The former every-fifth-block filter was removed without a bounded identity or cleanup strategy.
Why it matters
At six-second blocks, development chains accumulate about 14,400 Mapping rows and events per day, increasing database, snapshot, and indexing costs.
Evidence
- Related runtime area cnight_registration_state: Mock registration cadence and persistent CNight mapping state
- Area expectations: Emitting one registration every queried block has intentional, sustainable effects on Mapping storage and registration events.; Generated reward addresses and Dust keys satisfy downstream representation and semantic requirements.; Per-block mock identities vary sufficiently to avoid unintended registration ambiguity.; The mock remains within runtime UTXO capacity and weight assumptions.
- The mock always returns one UTXO.
- handle_registration inserts into Mapping and emits MappingAdded.
- Mapping is an unbounded StorageDoubleMap with no mock cleanup.
Reproduction or trace
Repeatedly process successive mock observations; each block adds one persistent Mapping entry.
Suggested fix
Bound the synthetic registration set or document and intentionally manage the resulting state growth.
Signed-off-by: Lech Głowiak <lech.glowiak@shielded.io>
Jina ReviewJina skipped this review. Review: https://app.usejina.com/reviews/c7bff964-2189-4e3b-a062-44813519f050
|
Signed-off-by: Lech Głowiak <lech.glowiak@shielded.io>
Jina ReviewJina has completed this review. Review: https://app.usejina.com/reviews/1b64b009-1e5a-4a36-a201-d83112c289a3
|
There was a problem hiding this comment.
Runtime Review - Merge Readiness 5/5
0 issues found - Ready to merge
Based only on accepted issues, Jina found none in the current investigation.
Earlier PR-thread findings are not accepted again because they are not current investigation candidates and are already represented in PR history.
Final review: No publishable candidates were produced by the current runtime investigation.
Prior findings are already present in PR history and are not reissued.
Review Comments
-
Runtime investigation produced no adjudicable candidates
All five investigation areas ended in warned status without tasks, tool calls, or final results.
The current candidate issue set is empty, so no new blocking runtime findings can be accepted in this review.
Evidence:
- All files under /tmp/jina-runtime-review-hiCKUh/investigations report status warned and issues: [].
- Candidate issues supplied for this review are [].
Related files:
primitives/mainchain-follower/src/data_source/cnight_observation_mock.rs
Issues
No medium/high-confidence runtime issues were found.
Summary
- Status: warned
- Areas investigated: 5
- Tasks/probes performed: 0
- Issues reported: 0
- Publishable issues: 0
- Inline comments: 0
- File-level comments: 0
- Review comments: 1
- Unanchored publishable findings posted as file-level comments: 0
- Blocked validations: 0
- Dashboard: full runtime review
- Commit: 9a61fd5
- Changed files: 2
- Tool calls: none
4 participants
Overview
PR #403 from branch moved to our remote, to let CI pass
Replace random UTXO generation with deterministic generation based on block number to ensure all nodes in a multi-node development network produce identical inherent data. This prevents block verification failures due to inherent data mismatches.
All nodes with the same block number now generate the same mock CNight observations (tx hashes, reward addresses, dust public keys), enabling multi-node development setups.
🗹 TODO before merging
📌 Submission Checklist
🧪 Testing Evidence
This fix enables running local multi-node networks in development mode:
Please describe any additional testing aside from CI:
🔱 Fork Strategy