feat(key-wallet-manager): WalletInterface::rewind_to_height with descendant cascade#169
Merged
Conversation
Add `demote_records_above`, `demote_record`, and (for the funds variant) `rebuild_utxos` so wallet-side reorg handling can demote above-cut records and rebuild the UTXO / spent-outpoint state from the surviving records. Mirrors the post-deserialization rebuild that the funds account's serde `Deserialize` impl already uses. The keys variant only needs the context demotions since it carries no UTXO state. Both helpers leave records currently in `InstantSend`, `Conflicted`, or `Abandoned` contexts alone.
…dant cascade Adds the wallet-level rewind orchestrator that consumes the new per-account demote / rebuild primitives. Validates the chainlock floor up front, runs an initial cut on every account, then drives a cross-account fixed-point loop that demotes any in-wallet record whose inputs spend an output of an already-demoted record. Finally rebuilds every funds account's UTXO state and rolls `last_processed_height` / `synced_height` back to `min(height, current)`. `used_addresses` markers are intentionally preserved as monotonic state. The trait method returns a `RewindOutcome` so the manager-level emitter can fire a single atomic event per wallet. Self-conflict detection and ISLock-quorum re-validation are explicitly deferred to follow-ups with in-code TODOs.
…t_transaction` Threads the wallet-side reorg path through the manager: - new `RewindResult` and `RewindError` types - `rewind_to_height` validates the chainlock floor across every managed wallet up front, then runs the per-wallet rewind and emits a single `WalletEvent::Reorg` per wallet whose state changed - `get_transaction` lookup for the auto-rebroadcast path - `WalletEvent::Reorg` variant with the partitioned demoted-vs-conflicted txid lists and the post-rewind balance The `&mut self` receiver on `rewind_to_height` is the lock contract: implementations must not yield mid-rewind so concurrent mutators on the same trait surface cannot interleave with a reorg-driven demotion. Mock implementations in `test_utils` get the minimum stubs needed to satisfy the trait.
Adds the exhaustive-match arm for the new `WalletEvent::Reorg` variant. The C ABI does not yet carry a dedicated reorg callback, so the arm logs intent via a TODO and drops the event. Wiring a real callback is tracked separately.
Adds seven scenarios behind the new `WalletInterface`: - single tx at H+1 demotes from `InBlock` to `Mempool` and emits a `WalletEvent::Reorg` with zero confirmed balance - TxA at H+1, TxB spending TxA at H+2 cascades both - IS-locked record is retained as `InstantSend` through the rewind - rewind below the chainlock floor is rejected with the correct error fields and a rewind at the floor itself is allowed - `last_processed_height` and `synced_height` roll back, and a rewind above the current height does not advance them forward - `get_transaction` returns stored records and `None` for unknown txids
- Import `RewindError` at module top in `event_tests.rs` instead of using `crate::wallet_interface::RewindError` inline - Use imported `WalletId` instead of `crate::WalletId` in `wallet_interface.rs` - Replace em-dash prose separators in doc comments with `:` or `;` - Remove caller-reference sentence from `spend_from` doc - Rewrite `state_changed` field doc to describe the field, not its callers
|
Manki — Review complete Planner (16s) Review — 12 findings Judge — 8 kept · 0 dropped (33s) Review metadataConfig:
Judge decisions:
Timing:
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## dev #169 +/- ##
==========================================
- Coverage 73.77% 73.65% -0.12%
==========================================
Files 325 325
Lines 74482 74694 +212
==========================================
+ Hits 54948 55018 +70
- Misses 19534 19676 +142
🚀 New features to boost your workflow:
|
![manki-review[bot]](https://avatars.githubusercontent.com/in/3200573?s=60&v=4){kind=small}
There was a problem hiding this comment.
Real blocker hiding in rebuild_utxos: sorting Mempool records (None < Some) before confirmed ones lets a demoted child re-spend a still-confirmed parent's UTXO that gets re-inserted unguarded — three independent reviewers landed on this same insert-without-spent-check bug, and there's no test that would catch it.
📊 8 findings (1 blocker, 2 warning, 4 suggestion, 1 nitpick) · 809 lines · 470s
Manki context
{
"meta": {
"prNumber": 169,
"commitSha": "91bc227e4eb329455b23538c6a96bc670028a14c",
"round": 1,
"timestamp": "2026-05-26T08:37:59.309Z",
"mankiVersion": "5.3.0",
"cap": {
"priorRoundCount": 0,
"maxAutoRounds": 5,
"skipCap": false,
"forceReview": false,
"bypassReason": "within_cap"
},
"trigger": {
"event": "pull_request:opened",
"sender": "xdustinface"
}
},
"config": {
"reviewLevel": "medium",
"memoryEnabled": false,
"reviewPasses": 1
},
"diff": {
"lines": 809,
"additions": 806,
"deletions": 3,
"filesReviewed": 10,
"fileTypes": {
".rs": 9,
".toml": 1
},
"oversizedHandled": false,
"perFile": [
{
"path": "dash-spv-ffi/src/callbacks.rs",
"additions": 8,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/Cargo.toml",
"additions": 1,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/event_tests.rs",
"additions": 188,
"deletions": 1,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/events.rs",
"additions": 43,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/process_block.rs",
"additions": 77,
"deletions": 1,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/test_utils/mock_wallet.rs",
"additions": 54,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/wallet_interface.rs",
"additions": 78,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"additions": 124,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/managed_account/managed_core_keys_account.rs",
"additions": 41,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs",
"additions": 192,
"deletions": 1,
"changeType": "modified"
}
]
},
"models": {
"planner": "claude-haiku-4-5",
"reviewer": "claude-sonnet-4-6",
"judge": "claude-opus-4-7",
"dedup": "claude-haiku-4-5"
},
"planner": {
"source": "planner",
"used": true,
"coreAgentInjections": [],
"priorRoundEffortDowngrades": [],
"teamSize": 4,
"reviewerEffort": "medium",
"judgeEffort": "high",
"prType": "feat",
"durationMs": 16077,
"language": "rust",
"context": "Dash HD wallet blockchain rewind",
"agents": [
{
"name": "Security & Safety",
"effort": "high"
},
{
"name": "Correctness & Logic",
"effort": "high"
},
{
"name": "Architecture & Design",
"effort": "medium"
},
{
"name": "Testing & Coverage",
"effort": "medium"
}
]
},
"reviewers": {
"agents": [
"Security & Safety",
"Correctness & Logic",
"Architecture & Design",
"Testing & Coverage"
],
"agentMetrics": [
{
"name": "Security & Safety",
"findingsRaw": 2,
"findingsKept": 2,
"durationMs": 373125,
"status": "success",
"responseLength": 4503,
"inputTokens": 3,
"outputTokens": 19806,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "high"
},
{
"name": "Correctness & Logic",
"findingsRaw": 2,
"findingsKept": 1,
"durationMs": 412972,
"status": "success",
"responseLength": 4205,
"inputTokens": 3,
"outputTokens": 23365,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "high"
},
{
"name": "Architecture & Design",
"findingsRaw": 4,
"findingsKept": 3,
"durationMs": 124358,
"status": "success",
"responseLength": 6760,
"inputTokens": 3,
"outputTokens": 6820,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "medium"
},
{
"name": "Testing & Coverage",
"findingsRaw": 4,
"findingsKept": 3,
"durationMs": 216621,
"status": "success",
"responseLength": 5804,
"inputTokens": 3,
"outputTokens": 12126,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "medium"
}
]
},
"judge": {
"summary": "Real blocker hiding in rebuild_utxos: sorting Mempool records (None < Some) before confirmed ones lets a demoted child re-spend a still-confirmed parent's UTXO that gets re-inserted unguarded — three independent reviewers landed on this same insert-without-spent-check bug, and there's no test that would catch it.",
"confidenceDistribution": {
"high": 6,
"medium": 2,
"low": 0
},
"severityChanges": 8,
"mergedDuplicates": 4,
"durationMs": 33093,
"retryCount": 0,
"verdictReason": "required_present",
"defensiveHardeningCount": 1,
"verdictTrace": {
"survivingBlockers": [
{
"file": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"title": "rebuild_utxos: wrong sort order leaves spent UTXOs in spendable set after rewind",
"fingerprint": "key-wallet/src/managed_account/managed_core_funds_account.rs:588:588:rebuild-utxos--wrong-sort-order-leaves-spent-UTXOs-in-spendable-set-after-rewind"
}
],
"novelWarnings": [],
"unresolvedPriors": []
},
"openThreadsState": "empty",
"openThreadCount": 0,
"resolvedThreadIdCount": 0,
"interRoundDiffState": "unknown",
"interRoundDiffTruncated": false
},
"dedup": {
"staticDropped": 0,
"llmDropped": 0,
"sameRoundLlmDropped": 4
},
"memory": {
"loadStatus": "disabled"
},
"findings": {
"count": 8,
"severityCounts": {
"blocker": 1,
"warning": 2,
"suggestion": 4,
"nitpick": 1
},
"entries": [
{
"fingerprint": {
"file": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"lineStart": 588,
"lineEnd": 588,
"slug": "rebuild-utxos--wrong-sort-order-leaves-spent-UTXOs-in-spendable-set-after-rewind"
},
"severity": "blocker",
"specialist": "Security & Safety",
"suggestedFix": "In the \u0060rebuild_utxos\u0060 inner output loop, add the same guard already present in \u0060update_utxos\u0060:\n\n\u0060\u0060\u0060rust\nlet outpoint = OutPoint { txid, vout: vout as u32 };\nif self.spent_outpoints.contains(&outpoint) {\n // Already spent by a previously-processed record in this rebuild.\n continue;\n}\nlet txout",
"title": "rebuild_utxos: wrong sort order leaves spent UTXOs in spendable set after rewind",
"judgeNotes": "Merged findings 1, 3, and 9 — same root cause. Three reviewers independently identified that sort_by_key on Option<height> puts None (Mempool/IS) before Some(h), so a demoted child spending a still-confirmed parent processes first, inserts the parent outpoint into spent_outpoints, then the parent's later insert re-adds the UTXO unconditionally. update_utxos already guards this path; rebuild_utxos must mirror it. Impact: Critical (inflated balance, double-spend attempts), Likelihood: Probable (an",
"judgeConfidence": "high",
"reachability": "reachable"
},
{
"fingerprint": {
"file": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"lineStart": 611,
"lineEnd": 611,
"slug": "rebuild-utxos--is-trusted-not-restored--causing-confirmed-balance-undercount-after-rewind"
},
"severity": "suggestion",
"specialist": "Security & Safety",
"suggestedFix": "Within the output loop in \u0060rebuild_utxos\u0060, reconstruct the trusted-output flag:\n\n\u0060\u0060\u0060rust\n// has_owned_input is true when any input of this tx spent a wallet UTXO\n// (which spent_outpoints already tracks from earlier records in this rebuild).\nlet has_owned_input = tx.input.iter()\n .any(|i| self.sp",
"title": "rebuild_utxos: is_trusted not restored, causing confirmed balance undercount after rewind",
"judgeNotes": "Conservative regression — never overstates funds, just shows trusted change as unconfirmed until reconfirmation. Single reviewer; downgrading from warning because the behavior is safe and the proposed fix requires adding new predicates (is_internal_address) that don't yet exist. Worth fixing but not shipping-blocking.",
"judgeConfidence": "medium",
"reachability": "reachable"
},
{
"fingerprint": {
"file": "key-wallet-manager/src/process_block.rs",
"lineStart": 337,
"lineEnd": 337,
"slug": "Partial-wallet-rewind-if-inner--BelowChainLockFloor--fires-after-earlier-wallets-already-mutated"
},
"severity": "nitpick",
"specialist": "Correctness & Logic",
"suggestedFix": "Collect outcomes before mutating:\n\u0060\u0060\u0060rust\nlet mut outcomes: Vec<(WalletId, RewindOutcome)> = Vec::new();\nfor wallet_id in &wallet_ids {\n let Some(info) = self.wallet_infos.get_mut(wallet_id) else { continue; };\n match info.rewind_to_height(height) {\n Ok(o) => outcomes.push((*wallet_id, ",
"title": "Partial wallet rewind if inner \u0060BelowChainLockFloor\u0060 fires after earlier wallets already mutated",
"judgeNotes": "Merged with finding 6 — both observe the inner Err arm is unreachable today given the outer pre-check and &mut self. Real concern is latent (a future refactor could enable partial mutation), not active. Downgrading from warning because the current code is correct; the recommendation is hardening (collect-then-apply, or debug_assert_unreachable!).",
"judgeConfidence": "high",
"reachability": "hypothetical",
"reachabilityReasoning": "The outer pre-check iterates the same wallet set under the same &mut self with an identical predicate, so any wallet that passes the pre-check cannot fail the inner check today.",
"tags": [
"defensive-hardening"
],
"originalSeverity": "suggestion"
},
{
"fingerprint": {
"file": "key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs",
"lineStart": 218,
"lineEnd": 218,
"slug": "Silent-no-op-default-for-rewind-to-height-is-dangerous-for-a-state-critical-operation"
},
"severity": "warning",
"specialist": "Architecture & Design",
"suggestedFix": "// In the trait definition — remove the default body:\nfn rewind_to_height(\n &mut self,\n height: CoreBlockHeight,\n) -> Result<RewindOutcome, RewindRejection>;",
"title": "Silent no-op default for rewind_to_height is dangerous for a state-critical operation",
"judgeNotes": "Real design hazard: a correctness-critical mutation with a structurally-successful default that bypasses the state_changed gate and skips Reorg emission. Single reviewer but the impact (silent data inconsistency on any future implementor) justifies keeping it as warning. Trivial fix — drop the default body.",
"judgeConfidence": "high",
"reachability": "unknown",
"reachabilityReasoning": "The trait is exported; reachability depends on whether any current or future implementor outside this PR's diff omits the override."
},
{
"fingerprint": {
"file": "key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs",
"lineStart": 490,
"lineEnd": 490,
"slug": "Descendant-cascade-re-collects-released-outpoints-from-the-entire-already-demoted-set-each-iteration-instead-of-just-the-frontier"
},
"severity": "suggestion",
"specialist": "Architecture & Design",
"suggestedFix": "let mut frontier: Vec<Txid> = demoted_txids.clone();\nloop {\n // Only collect outpoints released by the *previous* wave.\n let released_outpoints: BTreeSet<OutPoint> = {\n let mut set = BTreeSet::new();\n for account in self.accounts.all_accounts() {\n for txid in &frontier",
"title": "Descendant cascade re-collects released outpoints from the entire already_demoted set each iteration instead of just the frontier",
"judgeNotes": "Correct optimization observation; quadratic in spend-chain depth × accounts. Rewinds are rare and chain depth is typically small, so the practical impact is minimal. Suggestion is appropriate.",
"judgeConfidence": "high",
"reachability": "reachable"
},
{
"fingerprint": {
"file": "key-wallet/src/managed_account/managed_core_keys_account.rs",
"lineStart": 153,
"lineEnd": 153,
"slug": "demote-records-above-and-demote-record-are-duplicated-verbatim-across-ManagedCoreKeysAccount-and-ManagedCoreFundsAccount"
},
"severity": "suggestion",
"specialist": "Architecture & Design",
"suggestedFix": "// In ManagedAccountTrait or as a free function:\nfn demote_records_above(\n transactions: &mut BTreeMap<Txid, TransactionRecord>,\n height: CoreBlockHeight,\n) -> Vec<Txid> {\n let to_demote: Vec<Txid> = transactions\n .iter()\n .filter_map(|(txid, record)| match record.context.bloc",
"title": "demote_records_above and demote_record are duplicated verbatim across ManagedCoreKeysAccount and ManagedCoreFundsAccount",
"judgeNotes": "Real maintainability concern — any correctness fix to the filter (e.g. handling InstantSend) must be made in two places. Standard dedup suggestion; not blocking.",
"judgeConfidence": "high",
"reachability": "reachable"
},
{
"fingerprint": {
"file": "key-wallet-manager/src/event_tests.rs",
"lineStart": 1395,
"lineEnd": 1395,
"slug": "No-test-for-UTXO-restoration-when-parent-is-below-cut-and-child-is-above-cut"
},
"severity": "warning",
"specialist": "Testing & Coverage",
"suggestedFix": "Add a test:\n\u0060\u0060\u0060rust\n#[tokio::test]\nasync fn test_rewind_restores_utxo_when_child_above_cut() {\n let (mut manager, wallet_id, addr) = setup_manager_with_wallet();\n let wallets = BTreeSet::from([wallet_id]);\n\n // Parent confirmed at height 10.\n let tx_a = create_tx_paying_to(&addr, 0xe0);\n",
"title": "No test for UTXO restoration when parent is below cut and child is above cut",
"judgeNotes": "This is the test that would catch the blocker finding above — the cascade test only asserts demotion, never balance/UTXO state. Keeping at warning: missing coverage on the primary correctness invariant of rebuild_utxos.",
"judgeConfidence": "high",
"reachability": "reachable"
},
{
"fingerprint": {
"file": "key-wallet-manager/src/event_tests.rs",
"lineStart": 1438,
"lineEnd": 1438,
"slug": "No-test-verifying-Reorg-event-is-NOT-emitted-when-rewind-has-no-effect"
},
"severity": "suggestion",
"specialist": "Testing & Coverage",
"suggestedFix": "Extend \u0060test_rewind_below_current_does_not_advance_heights_upward\u0060 (or add a new test) to subscribe before rewinding and assert the channel is empty afterwards:\n\u0060\u0060\u0060rust\nlet mut rx = manager.subscribe_events();\nmanager.rewind_to_height(1000).await.expect(\"rewind succeeds\");\nlet reorg_events: Vec<_> =",
"title": "No test verifying Reorg event is NOT emitted when rewind has no effect",
"judgeNotes": "Behavioral contract (state_changed gate) is untested but the consequence of regression is moderate (spurious Reorg event). Downgrading from warning — low-noise gap, easy follow-up.",
"judgeConfidence": "medium",
"reachability": "reachable"
}
]
},
"usage": {
"inputTokens": 26,
"outputTokens": 65809,
"totalTokens": 65835,
"perStage": {
"planner": {
"inputTokens": 9,
"outputTokens": 1417,
"totalTokens": 1426
},
"reviewer": {
"inputTokens": 12,
"outputTokens": 62117,
"totalTokens": 62129
},
"judge": {
"inputTokens": 5,
"outputTokens": 2275,
"totalTokens": 2280
}
}
},
"verdict": "REQUEST_CHANGES",
"recap": {
"priorRoundCount": 0,
"reclassifiedPriorCount": 0
}
}Reviewed commit 91bc227
…is_trusted` Guards every UTXO insert against `spent_outpoints` so a record whose spending child sorts earlier (e.g. two records demoted to `Mempool` after a rewind, both with `None` heights) cannot resurrect a spent outpoint into the spendable set. Restores `is_trusted` on self-send change outputs by detecting `has_owned_input` against the rebuild's own `spent_outpoints` and matching against the account's internal pool. Post-rewind self-send change outputs that demote to `Mempool` continue to bucket into `confirmed` rather than silently undercounting. Addresses manki-review on PR [#169](#169) ([thread](#169 (comment)), [thread](#169 (comment))).
Removes the no-op default body of `WalletInfoInterface::rewind_to_height`. The default returned `Ok(RewindOutcome::default())` with `state_changed = false`, which silently suppresses the `WalletEvent::Reorg` event and leaves UTXOs unrebuilt — a correctness-critical mutation cannot be advisory. Any implementor that forgets to override would silently appear to succeed. Replaces the descendant cascade's full `already_demoted` re-scan with a `frontier` that carries only the previous wave. Any newly reachable descendant must spend an output of that wave, so the prior `O(D²)` loop collapses to `O(D)` for the outpoint collection step. The `already_demoted` set is retained for the candidate-skip predicate. Addresses manki-review on PR [#169](#169) ([thread](#169 (comment)), [thread](#169 (comment))).
…ys account `ManagedCoreFundsAccount::demote_records_above` and `demote_record` were verbatim duplicates of the keys-account versions that routed through `self.keys.transactions_mut()`. Collapse the funds versions to one-line delegations so a future correctness fix to the demotion predicate (e.g. also skipping `InstantSend`) only needs to land on the keys account. Addresses manki-review on PR [#169](#169) ([thread](#169 (comment))).
… on rewind Extends `test_rewind_cascades_to_descendant_in_same_account` to assert the post-rewind balance equals exactly the child output value. The parent UTXO remains claimed by the child (still in `Mempool`), so only the child's output reaches the spendable set. This pins down the order-independence of `rebuild_utxos`: without the `spent_outpoints` insert guard the parent UTXO would resurrect into the spendable set whenever the child's txid sorts before the parent's. Extends `test_rewind_below_current_does_not_advance_heights_upward` to subscribe to events before the no-op rewind and assert no `WalletEvent::Reorg` was emitted. This pins down the `state_changed` guard that suppresses spurious events. Addresses manki-review on PR [#169](#169) ([thread](#169 (comment)), [thread](#169 (comment))).
|
Manki — Review complete Planner (19s) Review — 5 findings Judge — 3 kept · 2 dropped (29s) Review metadataConfig:
Judge decisions:
Timing:
|
There was a problem hiding this comment.
The sort-order blocker is resolved by the spent-outpoint guard, but three reviewers independently flag that has_owned_input checks spent_outpoints instead of utxos — so is_trusted is effectively never set after rewind and the confirmed/unconfirmed split remains broken. The new tests cover the 2-level cascade and no-op Reorg suppression but don't lock in the is_trusted semantics or a 3-level cascade.
📊 3 findings (1 warning, 2 suggestion) · 828 lines · 263s
Manki context
{
"meta": {
"prNumber": 169,
"commitSha": "4128ef3a205d97df3f0d83b7365659a9d1a2519d",
"round": 2,
"timestamp": "2026-05-26T08:59:15.798Z",
"mankiVersion": "5.3.0",
"cap": {
"priorRoundCount": 1,
"maxAutoRounds": 5,
"skipCap": false,
"forceReview": false,
"bypassReason": "within_cap"
},
"trigger": {
"event": "pull_request:synchronize",
"sender": "xdustinface"
}
},
"config": {
"reviewLevel": "medium",
"memoryEnabled": false,
"reviewPasses": 1
},
"diff": {
"lines": 828,
"additions": 825,
"deletions": 3,
"filesReviewed": 10,
"fileTypes": {
".rs": 9,
".toml": 1
},
"oversizedHandled": false,
"perFile": [
{
"path": "dash-spv-ffi/src/callbacks.rs",
"additions": 8,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/Cargo.toml",
"additions": 1,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/event_tests.rs",
"additions": 209,
"deletions": 1,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/events.rs",
"additions": 43,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/process_block.rs",
"additions": 77,
"deletions": 1,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/test_utils/mock_wallet.rs",
"additions": 54,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/wallet_interface.rs",
"additions": 78,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"additions": 118,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/managed_account/managed_core_keys_account.rs",
"additions": 41,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs",
"additions": 196,
"deletions": 1,
"changeType": "modified"
}
]
},
"models": {
"planner": "claude-haiku-4-5",
"reviewer": "claude-sonnet-4-6",
"judge": "claude-opus-4-7",
"dedup": "claude-haiku-4-5"
},
"planner": {
"source": "planner",
"used": true,
"coreAgentInjections": [],
"priorRoundEffortDowngrades": [],
"teamSize": 4,
"reviewerEffort": "medium",
"judgeEffort": "medium",
"prType": "feat",
"durationMs": 19156,
"language": "rust",
"context": "blockchain wallet with FFI bindings",
"agents": [
{
"name": "Security & Safety",
"effort": "high"
},
{
"name": "Correctness & Logic",
"effort": "high"
},
{
"name": "Architecture & Design",
"effort": "medium"
},
{
"name": "Testing & Coverage",
"effort": "medium"
}
]
},
"reviewers": {
"agents": [
"Security & Safety",
"Correctness & Logic",
"Architecture & Design",
"Testing & Coverage"
],
"agentMetrics": [
{
"name": "Security & Safety",
"findingsRaw": 1,
"findingsKept": 1,
"durationMs": 203197,
"status": "success",
"responseLength": 2271,
"inputTokens": 3,
"outputTokens": 11596,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "high"
},
{
"name": "Correctness & Logic",
"findingsRaw": 1,
"findingsKept": 0,
"durationMs": 141388,
"status": "success",
"responseLength": 1271,
"inputTokens": 3,
"outputTokens": 7886,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "high"
},
{
"name": "Architecture & Design",
"findingsRaw": 1,
"findingsKept": 0,
"durationMs": 159188,
"status": "success",
"responseLength": 1980,
"inputTokens": 3,
"outputTokens": 8902,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "medium"
},
{
"name": "Testing & Coverage",
"findingsRaw": 2,
"findingsKept": 2,
"durationMs": 166025,
"status": "success",
"responseLength": 3488,
"inputTokens": 5,
"outputTokens": 4629,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "medium"
}
]
},
"judge": {
"summary": "The sort-order blocker is resolved by the spent-outpoint guard, but three reviewers independently flag that \u0060has_owned_input\u0060 checks \u0060spent_outpoints\u0060 instead of \u0060utxos\u0060 — so \u0060is_trusted\u0060 is effectively never set after rewind and the confirmed/unconfirmed split remains broken. The new tests cover the 2-level cascade and no-op Reorg suppression but don't lock in the \u0060is_trusted\u0060 semantics or a 3-level cascade.",
"confidenceDistribution": {
"high": 3,
"medium": 2,
"low": 0
},
"severityChanges": 5,
"mergedDuplicates": 0,
"durationMs": 28673,
"retryCount": 0,
"verdictReason": "novel_suggestion",
"threadEvaluations": [
{
"threadId": "PRRT_kwDOQSlaXs6Eu4Yb",
"status": "addressed",
"reason": "Lines 594-600 now add \u0060if self.spent_outpoints.contains(&outpoint) { continue; }\u0060, an order-independent guard that prevents spent UTXOs from being resurrected regardless of sort order."
},
{
"threadId": "PRRT_kwDOQSlaXs6Eu4Yt",
"status": "not_addressed",
"reason": "Line 615 sets \u0060utxo.is_trusted = has_owned_input && is_change\u0060, but \u0060has_owned_input\u0060 (line 581) checks \u0060self.spent_outpoints.contains(...)\u0060 — which holds inputs of already-processed records, not owned outputs. In a healthy chain this is always false, so \u0060is_trusted\u0060 is never set. Flagged by 3 independent reviewers."
},
{
"threadId": "PRRT_kwDOQSlaXs6Eu4Y1",
"status": "not_addressed",
"reason": "process_block.rs:334-345 still returns Err on inner BelowChainLockFloor mid-iteration after earlier wallets have been mutated. No defensive change in the inter-round diff."
},
{
"threadId": "PRRT_kwDOQSlaXs6Eu4ZI",
"status": "addressed",
"reason": "The trait default body was removed in wallet_info_interface.rs:224-228 — \u0060rewind_to_height\u0060 is now an unimplemented trait method, forcing each implementor to provide one."
},
{
"threadId": "PRRT_kwDOQSlaXs6Eu4ZP",
"status": "addressed",
"reason": "The cascade loop now maintains a \u0060frontier: Vec<Txid>\u0060 initialized to \u0060demoted_txids.clone()\u0060 and reassigned to \u0060new_demoted\u0060 each iteration; the inner loop iterates \u0060&frontier\u0060 instead of \u0060&already_demoted\u0060 (lines 486-545)."
},
{
"threadId": "PRRT_kwDOQSlaXs6Eu4Zc",
"status": "addressed",
"reason": "\u0060ManagedCoreFundsAccount::demote_records_above\u0060 and \u0060demote_record\u0060 now delegate to \u0060self.keys.demote_records_above(...)\u0060 / \u0060self.keys.demote_record(...)\u0060, removing the duplicated bodies."
},
{
"threadId": "PRRT_kwDOQSlaXs6Eu4Zn",
"status": "addressed",
"reason": "The cascade test now asserts \u0060balance.confirmed() + balance.unconfirmed() == TX_AMOUNT / 2\u0060 (lines 1361-1369), verifying that exactly the child's output is in the spendable set after the parent is restored and re-spent by the still-present child."
},
{
"threadId": "PRRT_kwDOQSlaXs6Eu4Z4",
"status": "addressed",
"reason": "\u0060test_rewind_below_current_does_not_advance_heights_upward\u0060 now subscribes to events before the no-op rewind, drains them, filters for \u0060WalletEvent::Reorg\u0060, and asserts the list is empty (lines 1449-1462)."
}
],
"verdictTrace": {
"survivingBlockers": [],
"novelWarnings": [
{
"file": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"title": "rebuild_utxos: has_owned_input uses wrong signal — is_trusted always false after rewind",
"fingerprint": "key-wallet/src/managed_account/managed_core_funds_account.rs:579:579:rebuild-utxos--has-owned-input-uses-wrong-signal---is-trusted-always-false-after-rewind"
}
],
"unresolvedPriors": []
},
"openThreadsState": "fetched",
"openThreadCount": 8,
"resolvedThreadIdCount": 0,
"interRoundDiffState": "changed",
"interRoundDiffBytes": 8227,
"interRoundDiffTruncated": false
},
"dedup": {
"staticDropped": 0,
"llmDropped": 0,
"durationMs": 1
},
"memory": {
"loadStatus": "disabled"
},
"findings": {
"count": 3,
"severityCounts": {
"blocker": 0,
"warning": 1,
"suggestion": 2,
"nitpick": 0
},
"entries": [
{
"fingerprint": {
"file": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"lineStart": 579,
"lineEnd": 579,
"slug": "rebuild-utxos--has-owned-input-uses-wrong-signal---is-trusted-always-false-after-rewind"
},
"severity": "warning",
"specialist": "Security & Safety",
"suggestedFix": "// Option A – reverse sort so parents always precede children, then use utxos:\nrecords.sort_by_key(|r| r.context.block_info().map(|i| i.height()).unwrap_or(u32::MAX));\n// Then inside the loop, before inserting outputs:\nlet has_owned_input = tx.input.iter().any(|input| self.utxos.contains_key(&input.",
"title": "rebuild_utxos: has_owned_input uses wrong signal — is_trusted always false after rewind",
"judgeNotes": "Three independent reviewers converge: \u0060spent_outpoints\u0060 holds inputs of already-processed records, not our owned outputs, so \u0060has_owned_input\u0060 is false in a healthy chain and \u0060is_trusted\u0060 never gets restored. Impact: confirmed balance undercount after rewind. Likelihood: Probable on any rewind with self-spend change. Consensus signal is strong.",
"judgeConfidence": "high",
"reachability": "reachable"
},
{
"fingerprint": {
"file": "key-wallet-manager/src/event_tests.rs",
"lineStart": 1353,
"lineEnd": 1353,
"slug": "No-test-for-3-level-cascade---frontier-optimization-is-unverified-past-wave-1"
},
"severity": "suggestion",
"specialist": "Testing & Coverage",
"suggestedFix": "#[tokio::test]\nasync fn test_rewind_cascades_three_levels() {\n let (mut manager, wallet_id, addr) = setup_manager_with_wallet();\n let wallets = BTreeSet::from([wallet_id]);\n\n let tx_a = create_tx_paying_to(&addr, 0xd0);\n manager.process_block_for_wallets(&make_block(vec![tx_a.clone()], 0",
"title": "No test for 3-level cascade — frontier optimization is unverified past wave 1",
"judgeNotes": "Valid coverage gap: the frontier change only exercises wave 1 in tests, so a regression in \u0060frontier = new_demoted\u0060 for wave 2+ would go undetected. Impact: Medium, Likelihood: Possible. Downgraded from warning since the cascade logic is simple and only one reviewer flagged it.",
"judgeConfidence": "medium",
"reachability": "reachable"
},
{
"fingerprint": {
"file": "key-wallet-manager/src/event_tests.rs",
"lineStart": 1362,
"lineEnd": 1362,
"slug": "Cascade-balance-assertion-elides-confirmed-unconfirmed-split--leaving--is-trusted--fix-unverified"
},
"severity": "suggestion",
"specialist": "Testing & Coverage",
"suggestedFix": "let balance = info.balance();\n// tx_b spends our own UTXO and pays change back to us: is_trusted must be set\n// by rebuild_utxos so the output lands in confirmed(), not unconfirmed().\nassert_eq!(\n balance.confirmed(),\n TX_AMOUNT / 2,\n \"trusted mempool change must be credited to confirmed() ",
"title": "Cascade balance assertion elides confirmed/unconfirmed split, leaving \u0060is_trusted\u0060 fix unverified",
"judgeNotes": "Reasonable test improvement that would have caught Finding 1. Keep as suggestion per the reviewer's call.",
"judgeConfidence": "medium",
"reachability": "reachable"
}
]
},
"usage": {
"inputTokens": 28,
"outputTokens": 36840,
"totalTokens": 36868,
"perStage": {
"planner": {
"inputTokens": 9,
"outputTokens": 1793,
"totalTokens": 1802
},
"reviewer": {
"inputTokens": 14,
"outputTokens": 33013,
"totalTokens": 33027
},
"judge": {
"inputTokens": 5,
"outputTokens": 2034,
"totalTokens": 2039
},
"dedup": {
"inputTokens": 0,
"outputTokens": 0,
"totalTokens": 0
}
}
},
"verdict": "REQUEST_CHANGES",
"recap": {
"priorRoundCount": 1,
"reclassifiedPriorCount": 0
}
}Reviewed commit 4128ef3
|
Review skipped for
|
…_utxos` `has_owned_input` was checking `spent_outpoints`, which only sees inputs of already-processed records. A mempool child sorted before its block parent would observe an empty set, so `is_trusted` never got restored and trusted change still landed in `unconfirmed` after rewind. Collect every owned outpoint from the active records in a pre-pass and use that set for the check, decoupling the signal from sort order. Addresses manki review comment on PR #169 #169 (comment)
…restoration on rewind Add `test_rewind_cascades_three_levels` so a regression that drops descendants past the first wave of the frontier cascade is caught by tests, and `test_rewind_restores_is_trusted_on_mempool_change` to lock in the `has_owned_input` fix by paying change back to an internal-pool address and asserting the demoted output lands in `confirmed()` rather than `unconfirmed()`. Addresses manki review comments on PR #169 #169 (comment) #169 (comment)
|
Manki — Review complete Planner (18s) Review — 3 findings Judge — 1 kept · 0 dropped (81s) Review metadataConfig:
Judge decisions:
Timing:
|
There was a problem hiding this comment.
Previous three open threads all addressed: the owned-outpoints prepass replaces the bogus spent_outpoints signal, and two new tests cover the 3-level cascade and is_trusted restoration. One new suggestion about adding a balance assertion to the 3-level test — reasonable but not blocking.
Blocking unresolved review threads:
- [Blocker] rebuild_utxos: wrong sort order leaves spent UTXOs in spendable set after rewind —
key-wallet/src/managed_account/managed_core_funds_account.rs:588 - [Warning] Silent no-op default for rewind_to_height is dangerous for a state-critical operation —
key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs:218 - [Warning] No test for UTXO restoration when parent is below cut and child is above cut —
key-wallet-manager/src/event_tests.rs:1395 - [Warning] rebuild_utxos: has_owned_input uses wrong signal — is_trusted always false after rewind —
key-wallet/src/managed_account/managed_core_funds_account.rs:579
📊 1 findings (1 suggestion) · 913 lines · 381s
Manki context
{
"meta": {
"prNumber": 169,
"commitSha": "746743103f281d67160ef4dd2e37f727d08938eb",
"round": 3,
"timestamp": "2026-05-26T09:15:17.189Z",
"mankiVersion": "5.3.0",
"cap": {
"priorRoundCount": 2,
"maxAutoRounds": 5,
"skipCap": false,
"forceReview": false,
"bypassReason": "within_cap"
},
"trigger": {
"event": "pull_request:synchronize",
"sender": "xdustinface"
}
},
"config": {
"reviewLevel": "medium",
"memoryEnabled": false,
"reviewPasses": 1
},
"diff": {
"lines": 913,
"additions": 910,
"deletions": 3,
"filesReviewed": 10,
"fileTypes": {
".rs": 9,
".toml": 1
},
"oversizedHandled": false,
"perFile": [
{
"path": "dash-spv-ffi/src/callbacks.rs",
"additions": 8,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/Cargo.toml",
"additions": 1,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/event_tests.rs",
"additions": 272,
"deletions": 1,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/events.rs",
"additions": 43,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/process_block.rs",
"additions": 77,
"deletions": 1,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/test_utils/mock_wallet.rs",
"additions": 54,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/wallet_interface.rs",
"additions": 78,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"additions": 140,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/managed_account/managed_core_keys_account.rs",
"additions": 41,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs",
"additions": 196,
"deletions": 1,
"changeType": "modified"
}
]
},
"models": {
"planner": "claude-haiku-4-5",
"reviewer": "claude-sonnet-4-6",
"judge": "claude-opus-4-7",
"dedup": "claude-haiku-4-5"
},
"planner": {
"source": "planner",
"used": true,
"coreAgentInjections": [],
"priorRoundEffortDowngrades": [],
"teamSize": 4,
"reviewerEffort": "medium",
"judgeEffort": "medium",
"prType": "feat",
"durationMs": 18070,
"language": "rust",
"context": "Dash cryptocurrency wallet library",
"agents": [
{
"name": "Security & Safety",
"effort": "high"
},
{
"name": "Correctness & Logic",
"effort": "high"
},
{
"name": "Architecture & Design",
"effort": "medium"
},
{
"name": "Testing & Coverage",
"effort": "medium"
}
]
},
"reviewers": {
"agents": [
"Security & Safety",
"Correctness & Logic",
"Architecture & Design",
"Testing & Coverage"
],
"agentMetrics": [
{
"name": "Security & Safety",
"findingsRaw": 0,
"findingsKept": 0,
"durationMs": 164161,
"status": "success",
"responseLength": 2,
"inputTokens": 3,
"outputTokens": 9211,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "high"
},
{
"name": "Correctness & Logic",
"findingsRaw": 1,
"findingsKept": 0,
"durationMs": 269863,
"status": "success",
"responseLength": 2393,
"inputTokens": 3,
"outputTokens": 16053,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "high"
},
{
"name": "Architecture & Design",
"findingsRaw": 0,
"findingsKept": 0,
"durationMs": 84702,
"status": "success",
"responseLength": 2,
"inputTokens": 3,
"outputTokens": 4561,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "medium"
},
{
"name": "Testing & Coverage",
"findingsRaw": 2,
"findingsKept": 1,
"durationMs": 157718,
"status": "success",
"responseLength": 3758,
"inputTokens": 3,
"outputTokens": 9031,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "medium"
}
]
},
"judge": {
"summary": "Previous three open threads all addressed: the owned-outpoints prepass replaces the bogus spent_outpoints signal, and two new tests cover the 3-level cascade and is_trusted restoration. One new suggestion about adding a balance assertion to the 3-level test — reasonable but not blocking.",
"confidenceDistribution": {
"high": 0,
"medium": 1,
"low": 0
},
"severityChanges": 1,
"mergedDuplicates": 0,
"durationMs": 80971,
"retryCount": 0,
"verdictReason": "prior_unaddressed",
"threadEvaluations": [
{
"threadId": "PRRT_kwDOQSlaXs6EvRUs",
"status": "addressed",
"reason": "The diff replaces \u0060self.spent_outpoints.contains(...)\u0060 with \u0060owned_outpoints.contains(...)\u0060, where owned_outpoints is built via an up-front prepass over all record outputs filtered by \u0060account_type.contains_address\u0060, making has_owned_input a correct order-independent ownership signal (lines 580-603)."
},
{
"threadId": "PRRT_kwDOQSlaXs6EvRVB",
"status": "addressed",
"reason": "New test \u0060test_rewind_cascades_three_levels\u0060 constructs a tx_a→tx_b→tx_c chain and asserts all three txids appear in demoted_txids, exercising wave 2 of the frontier propagation."
},
{
"threadId": "PRRT_kwDOQSlaXs6EvRVK",
"status": "addressed",
"reason": "New test \u0060test_rewind_restores_is_trusted_on_mempool_change\u0060 asserts \u0060balance.confirmed() == TX_AMOUNT / 2\u0060 and \u0060balance.unconfirmed() == 0\u0060, directly verifying the is_trusted restoration in rebuild_utxos."
}
],
"verdictTrace": {
"survivingBlockers": [],
"novelWarnings": [],
"unresolvedPriors": [
{
"file": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"title": "rebuild_utxos: wrong sort order leaves spent UTXOs in spendable set after rewind",
"fingerprint": "key-wallet/src/managed_account/managed_core_funds_account.rs:588:588:rebuild-utxos--wrong-sort-order-leaves-spent-UTXOs-in-spendable-set-after-rewind",
"round": 1,
"line": 588,
"severity": "blocker"
},
{
"file": "key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs",
"title": "Silent no-op default for rewind_to_height is dangerous for a state-critical operation",
"fingerprint": "key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs:218:218:Silent-no-op-default-for-rewind-to-height-is-dangerous-for-a-state-critical-operation",
"round": 1,
"line": 218,
"severity": "warning"
},
{
"file": "key-wallet-manager/src/event_tests.rs",
"title": "No test for UTXO restoration when parent is below cut and child is above cut",
"fingerprint": "key-wallet-manager/src/event_tests.rs:1395:1395:No-test-for-UTXO-restoration-when-parent-is-below-cut-and-child-is-above-cut",
"round": 1,
"line": 1395,
"severity": "warning"
},
{
"file": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"title": "rebuild_utxos: has_owned_input uses wrong signal — is_trusted always false after rewind",
"fingerprint": "key-wallet/src/managed_account/managed_core_funds_account.rs:579:579:rebuild-utxos--has-owned-input-uses-wrong-signal---is-trusted-always-false-after-rewind",
"round": 2,
"line": 579,
"severity": "warning"
}
]
},
"openThreadsState": "fetched",
"openThreadCount": 3,
"resolvedThreadIdCount": 8,
"interRoundDiffState": "changed",
"interRoundDiffBytes": 5335,
"interRoundDiffTruncated": false
},
"dedup": {
"staticDropped": 0,
"llmDropped": 2,
"duplicateMatches": [
{
"droppedTitle": "test_rewind_cascades_three_levels tests height-filter, not cascade propagation",
"matchedTitle": "Descendant cascade re-collects released outpoints from the entire already_demoted set each iteration instead of just the frontier",
"matchType": "llm"
},
{
"droppedTitle": "test_rewind_restores_is_trusted_on_mempool_change may not exercise the ordering-dependent failure path",
"matchedTitle": "rebuild_utxos: is_trusted not restored, causing confirmed balance undercount after rewind",
"matchType": "llm"
}
],
"durationMs": 68055
},
"memory": {
"loadStatus": "disabled"
},
"findings": {
"count": 1,
"severityCounts": {
"blocker": 0,
"warning": 0,
"suggestion": 1,
"nitpick": 0
},
"entries": [
{
"fingerprint": {
"file": "key-wallet-manager/src/event_tests.rs",
"lineStart": 1371,
"lineEnd": 1371,
"slug": "test-rewind-cascades-three-levels-asserts-txid-demotion-but-not-post-rewind-balance"
},
"severity": "suggestion",
"specialist": "Testing & Coverage",
"suggestedFix": "let info = manager.get_wallet_info(&wallet_id).expect(\"wallet present\");\nlet balance = info.balance();\n// After rewind all three txs are in Mempool; only tx_a's output\n// is unspent (tx_b spends it, tx_c spends tx_b's output).\n// tx_c's output (TX_AMOUNT/4) is the surviving UTXO; tx_a's input\n// isn",
"title": "test_rewind_cascades_three_levels asserts txid demotion but not post-rewind balance",
"judgeNotes": "Impact: Low (test coverage gap), Likelihood: Possible (regression in deeper cascade UTXO arithmetic would slip past). The companion is_trusted test covers the rebuild correctness on a 2-level chain; adding a balance check here is a reasonable belt-and-suspenders but not required.",
"judgeConfidence": "medium",
"reachability": "reachable"
}
]
},
"usage": {
"inputTokens": 36,
"outputTokens": 50222,
"totalTokens": 50258,
"perStage": {
"planner": {
"inputTokens": 9,
"outputTokens": 1692,
"totalTokens": 1701
},
"reviewer": {
"inputTokens": 12,
"outputTokens": 38856,
"totalTokens": 38868
},
"judge": {
"inputTokens": 5,
"outputTokens": 694,
"totalTokens": 699
},
"dedup": {
"inputTokens": 10,
"outputTokens": 8980,
"totalTokens": 8990
}
}
},
"verdict": "REQUEST_CHANGES",
"recap": {
"priorRoundCount": 2,
"reclassifiedPriorCount": 0
}
}Reviewed commit 7467431
|
Review skipped for
|
…ascade Adds a `confirmed() + unconfirmed()` assertion to `test_rewind_cascades_three_levels` so a `rebuild_utxos` regression that double-removes the intermediate `tx_b`'s output or resurrects a spent ancestor is caught by the test, not just frontier-propagation regressions. Addresses manki-review review comment on PR #169 #169 (comment)
|
Manki — Review complete Planner (34s) Review — 0 findings Judge — 0 kept · 0 dropped (5s) Review metadataConfig:
Timing:
|
There was a problem hiding this comment.
No new findings from this round's reviewers — the prior cascade and is_trusted concerns appear settled and nothing new surfaced.
📊 0 findings (none) · 926 lines · 134s
Manki context
{
"meta": {
"prNumber": 169,
"commitSha": "7e7bc4d1319a5e8a91675a27ee2432b02b648240",
"round": 4,
"timestamp": "2026-05-26T09:23:50.422Z",
"mankiVersion": "5.3.0",
"cap": {
"priorRoundCount": 3,
"maxAutoRounds": 5,
"skipCap": false,
"forceReview": false,
"bypassReason": "within_cap"
},
"trigger": {
"event": "pull_request:synchronize",
"sender": "xdustinface"
}
},
"config": {
"reviewLevel": "medium",
"memoryEnabled": false,
"reviewPasses": 1
},
"diff": {
"lines": 926,
"additions": 923,
"deletions": 3,
"filesReviewed": 10,
"fileTypes": {
".rs": 9,
".toml": 1
},
"oversizedHandled": false,
"perFile": [
{
"path": "dash-spv-ffi/src/callbacks.rs",
"additions": 8,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/Cargo.toml",
"additions": 1,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/event_tests.rs",
"additions": 285,
"deletions": 1,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/events.rs",
"additions": 43,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/process_block.rs",
"additions": 77,
"deletions": 1,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/test_utils/mock_wallet.rs",
"additions": 54,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet-manager/src/wallet_interface.rs",
"additions": 78,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/managed_account/managed_core_funds_account.rs",
"additions": 140,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/managed_account/managed_core_keys_account.rs",
"additions": 41,
"deletions": 0,
"changeType": "modified"
},
{
"path": "key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs",
"additions": 196,
"deletions": 1,
"changeType": "modified"
}
]
},
"models": {
"planner": "claude-haiku-4-5",
"reviewer": "claude-sonnet-4-6",
"judge": "claude-opus-4-7",
"dedup": "claude-haiku-4-5"
},
"planner": {
"source": "planner",
"used": true,
"coreAgentInjections": [],
"priorRoundEffortDowngrades": [],
"teamSize": 3,
"reviewerEffort": "medium",
"judgeEffort": "high",
"prType": "feat",
"durationMs": 33681,
"language": "rust",
"context": "Cryptocurrency wallet library (Dash)",
"agents": [
{
"name": "Correctness & Logic",
"effort": "high"
},
{
"name": "Testing & Coverage",
"effort": "high"
},
{
"name": "Architecture & Design",
"effort": "medium"
}
]
},
"reviewers": {
"agents": [
"Security & Safety",
"Correctness & Logic",
"Architecture & Design",
"Testing & Coverage"
],
"agentMetrics": [
{
"name": "Security & Safety",
"findingsRaw": 0,
"findingsKept": 0,
"durationMs": 41258,
"status": "success",
"responseLength": 2,
"inputTokens": 3,
"outputTokens": 1927,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "medium"
},
{
"name": "Correctness & Logic",
"findingsRaw": 0,
"findingsKept": 0,
"durationMs": 82874,
"status": "success",
"responseLength": 2,
"inputTokens": 3,
"outputTokens": 4310,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "high"
},
{
"name": "Architecture & Design",
"findingsRaw": 0,
"findingsKept": 0,
"durationMs": 28823,
"status": "success",
"responseLength": 2,
"inputTokens": 3,
"outputTokens": 1314,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "medium"
},
{
"name": "Testing & Coverage",
"findingsRaw": 0,
"findingsKept": 0,
"durationMs": 77959,
"status": "success",
"responseLength": 2,
"inputTokens": 3,
"outputTokens": 3667,
"retryCount": 0,
"model": "claude-sonnet-4-6",
"effort": "high"
}
]
},
"judge": {
"summary": "No new findings from this round's reviewers — the prior cascade and is_trusted concerns appear settled and nothing new surfaced.",
"confidenceDistribution": {
"high": 0,
"medium": 0,
"low": 0
},
"severityChanges": 0,
"mergedDuplicates": 0,
"durationMs": 4522,
"retryCount": 0,
"verdictReason": "only_nit_or_suggestion",
"verdictTrace": {
"survivingBlockers": [],
"novelWarnings": [],
"unresolvedPriors": []
},
"openThreadsState": "fetched",
"openThreadCount": 0,
"resolvedThreadIdCount": 12,
"interRoundDiffState": "changed",
"interRoundDiffBytes": 1174,
"interRoundDiffTruncated": false
},
"dedup": {
"staticDropped": 0,
"llmDropped": 0
},
"memory": {
"loadStatus": "disabled"
},
"findings": {
"count": 0,
"severityCounts": {
"blocker": 0,
"warning": 0,
"suggestion": 0,
"nitpick": 0
},
"entries": []
},
"usage": {
"inputTokens": 26,
"outputTokens": 14410,
"totalTokens": 14436,
"perStage": {
"planner": {
"inputTokens": 9,
"outputTokens": 3086,
"totalTokens": 3095
},
"reviewer": {
"inputTokens": 12,
"outputTokens": 11218,
"totalTokens": 11230
},
"judge": {
"inputTokens": 5,
"outputTokens": 106,
"totalTokens": 111
}
}
},
"verdict": "APPROVE",
"recap": {
"priorRoundCount": 3,
"reclassifiedPriorCount": 0
}
}Reviewed commit 7e7bc4d
|
Review skipped for
|
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
2 participants
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
Phase 4b of #137. Builds on the
Conflicted/Abandonedvariants from #144 (merged in #168).WalletInterface::rewind_to_height(&mut self, height)returningRewindResult { demoted_txids, conflicted_txids }.&mut selfmakes the lock contract compile-time.WalletInterface::get_transaction(txid)for the auto-rebroadcast path in #146.ManagedAccountTraitgainsrewind_to_heightwith a fixed-point descendant cascade: walkstransactionsin reverse height order, demotes each record above the fork height, then iteratively demotes any in-accountTxBwhose inputs spend a demoted record's outputs.spent_outpointsrebuild from the rewind point (full rebuild for v1, optimise later if profiling justifies).last_processed_heightandsynced_heighttomin(h, current), per-wallet and aggregated.h < last_applied_chain_lock.height, returnsRewindError::BelowChainLockFloor.WalletEvent::Reorg { fork_height, demoted_txids, conflicted_txids }.dash-spv-ffi/src/callbacks.rshandles the new event (full FFI callback surface deferred — TODO).key-wallet-managertests cover single demotion, descendant chains, IS retention, chainlock floor refuse, persisted rollback, get_transaction lookup, lock contract.Deferred (per issue scope)
dash-spv. Records currently inInstantSend(islock)are retained through the cut-by-height filter (the safer half is automatic). Demoting an IS record whose signing quorum no longer exists post-reorg requires SPV-side wiring driven viaprocess_mempool_transaction(tx, None). TODO comment inwallet_info_interface.rs.SyncEvent::ChainReorg→rewind_to_heightplumbing lives in a follow-up — not in this PR's task list. Existing SPV reorg handlers don't call into the wallet today, so adding the method doesn't break downstream.Conflictedrather thanMempoolwhen a new-chain block spends the same input as a demoted outgoing tx) deferred to #146. TODO comment in code.WalletEvent::Reorgscoped out — exhaustive match arm consumes/drops the event with a TODO so the C ABI keeps compiling.Verification
cargo fmt --checkcleancargo clippy --all-targets -p key-wallet -p key-wallet-manager -p dash-spv -p dash-spv-ffi -- -D warningscleancargo test --lib -p key-wallet -p key-wallet-manager -p dash-spvall pass (487 + 53 + 538)pre-commit run --all-files --hook-stage pre-pushall hooks passCloses #145.