fix(platform-wallet): stale-islock to chainlock fallback for the L1 invite claim - #4364
Merged
Merged
Conversation
…e claim (#4240) A voucher islock is signed at creation, so by claim time (minutes-to-hours later, possibly across a testnet platform-4.1 quorum rotation) Drive may reject it with InvalidInstantAssetLockProofSignatureError ("try chain asset lock proof instead"). claim_invitation now recovers exactly as the register/top-up paths do: - reconstruct_asset_lock_proof / assemble_asset_lock_proof return a ReconstructedProof { primary, chain_fallback }. When the link carried an islock AND the funding tx is already chain-locked, a ChainLock proof over the SAME credit output is built as chain_fallback. - The primary proof is submitted through submit_with_cl_height_retry. If it is an IS proof rejected with is_instant_lock_proof_invalid(), the ChainLock fallback is resubmitted over the same outpoint. If no fallback exists (tx not yet chain-locked), AssetLockNotChainLocked surfaces a clear retry signal. Adds unit tests for the three assemble outcomes (chainlock-only, islock+chainlocked→fallback, islock+not-chainlocked→no-fallback). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…L resubmission arm The unit tests pin proof assembly (IS primary + optional CL fallback) but not the resubmission arm itself, which needs an injectable submission seam to be unit-testable; note that at the arm so the gap raised in review is visible in the code. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
bfoss765
requested review from
QuantumExplorer,
ZocoLini,
lklimek,
llbartekll and
shumkov
as code owners
August 10, 2026 21:19
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughInvitation proof reconstruction now returns a primary proof and an optional ChainLock fallback. Claims retry stale InstantSend submissions with the fallback. ChainLock-only invitations use a ChainLock primary proof, and identity derivation uses the primary proof. ChangesInvitation proof flow
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant InvitationClaim
participant ProofReconstruction
participant Platform
InvitationClaim->>ProofReconstruction: reconstruct primary and fallback proofs
ProofReconstruction-->>InvitationClaim: return ReconstructedProof
InvitationClaim->>Platform: submit primary proof with chain-height retry
Platform-->>InvitationClaim: reject stale InstantSend proof
InvitationClaim->>Platform: submit ChainLock fallback proof
Platform-->>InvitationClaim: return claim result
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Collaborator
|
🕓 Ready for review — next in queue (commit df7c0a0) |
…d path The salvaged fallback changes reconstruct/assemble to return a ReconstructedProof carrying the optional ChainLock fallback; #4332's prospective-identity-id caller and its two tests derive the id from the PRIMARY proof (both proofs cover the same credit output, so the id is identical either way). 619 tests green. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Final validation — Codex/Sol only (Phase 2 disabled)
The invitation proof reconstruction and stale-InstantSend-to-ChainLock fallback are internally consistent, and no blocking correctness issue was found. The central resubmission control flow remains untested, so this review requests direct coverage before regressions can surface only in live invitation claims.
Source: codex general reviewer (gpt-5.6-sol), codex rust-quality reviewer (gpt-5.6-sol), and final codex verifier (gpt-5.6-sol); openclaw-agent/cliproxy/gpt-5.6-sol is orchestration-only and not reviewer evidence.
Validated zero-blocker Codex/Sol precheck evidence was promoted to final because Phase 2 (Sonnet/Opus) is temporarily disabled. This is Codex/Sol-only final validation, not Codex + Sonnet/Opus coverage.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet/Opus: not run (Phase 2 disabled — temporary Codex/Sol-only final)
- Secondary pass: disabled (
temporary_phase2_sonnet_disable)
🟡 1 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `packages/rs-platform-wallet/src/wallet/identity/network/invitation.rs`:
- [SUGGESTION] packages/rs-platform-wallet/src/wallet/identity/network/invitation.rs:539-584: Add direct coverage for the stale-islock resubmission branch
The new tests verify proof assembly, including when a ChainLock fallback is available, but they never execute this two-attempt submission flow. They would therefore continue passing if a stale InstantSend rejection stopped submitting the fallback, an unrelated SDK error incorrectly triggered a second submission, a successful primary submission invoked the fallback, or the no-fallback path stopped returning `AssetLockNotChainLocked`. Extract this orchestration behind an injectable async submission seam, similar to `fetch_funding_tx_with_retry`, and cover those four outcomes so the PR's principal behavior does not depend exclusively on live claims.
Comment on lines
+539
to
+584
| let identity = match submit_with_cl_height_retry(settings, |s| { | ||
| placeholder.put_to_platform_and_wait_for_response_with_private_key( | ||
| &self.sdk, | ||
| asset_lock, | ||
| primary.clone(), | ||
| &voucher_priv.0, | ||
| identity_signer, | ||
| settings, | ||
| s, | ||
| ) | ||
| .await | ||
| .map_err(PlatformWalletError::Sdk)?; | ||
| }) | ||
| .await | ||
| { | ||
| Ok(identity) => identity, | ||
| // Coverage note (#4240 review): the unit tests pin proof assembly | ||
| // (IS primary + optional CL fallback) but not this resubmission arm | ||
| // itself — exercising it needs an injectable submission seam, so | ||
| // until one exists regressions here surface only in live claims. | ||
| Err(e) if is_instant_lock_proof_invalid(&e) => { | ||
| let Some(chain_proof) = chain_fallback else { | ||
| // The islock was rejected but the funding tx is not yet | ||
| // chain-locked, so no ChainLock proof can be built. Surface a | ||
| // clear retry signal rather than the raw consensus error. | ||
| return Err(PlatformWalletError::AssetLockNotChainLocked( | ||
| "invitation islock proof was rejected by Platform (stale — quorum \ | ||
| rotated or no longer recent) and the funding transaction is not yet \ | ||
| chain-locked, so no ChainLock fallback is possible; retry once the \ | ||
| funding block is chain-locked" | ||
| .to_string(), | ||
| )); | ||
| }; | ||
| tracing::warn!( | ||
| "invitation IS-lock proof rejected by Platform on claim; retrying with a \ | ||
| ChainLock proof over the same funding outpoint" | ||
| ); | ||
| submit_with_cl_height_retry(settings, |s| { | ||
| placeholder.put_to_platform_and_wait_for_response_with_private_key( | ||
| &self.sdk, | ||
| chain_proof.clone(), | ||
| &voucher_priv.0, | ||
| identity_signer, | ||
| s, | ||
| ) | ||
| }) | ||
| .await | ||
| .map_err(PlatformWalletError::Sdk)? | ||
| } | ||
| Err(e) => return Err(PlatformWalletError::Sdk(e)), |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: Add direct coverage for the stale-islock resubmission branch
The new tests verify proof assembly, including when a ChainLock fallback is available, but they never execute this two-attempt submission flow. They would therefore continue passing if a stale InstantSend rejection stopped submitting the fallback, an unrelated SDK error incorrectly triggered a second submission, a successful primary submission invoked the fallback, or the no-fallback path stopped returning AssetLockNotChainLocked. Extract this orchestration behind an injectable async submission seam, similar to fetch_funding_tx_with_retry, and cover those four outcomes so the PR's principal behavior does not depend exclusively on live claims.
source: ['codex']
… injectable seam The claim's two-attempt submission flow (primary proof, then the ChainLock fallback on Platform's stale-islock rejection) was pinned only by proof-assembly tests, so a regression in the orchestration itself — submitting the fallback for an unrelated error, skipping it for a stale islock, or losing the AssetLockNotChainLocked retry signal — would surface only in live claims. Extract the orchestration into submit_claim_with_stale_islock_fallback, generic over the submit future (mirroring fetch_funding_tx_with_retry's seam), and script it directly: - a successful primary never touches the fallback - a stale-islock rejection resubmits the ChainLock fallback - a stale-islock rejection with no fallback surfaces AssetLockNotChainLocked - an unrelated SDK error propagates without a second submission - the recovery runs exactly once (a rejected fallback is not re-recovered) claim_invitation now routes through the seam unchanged in behavior; the coverage-boundary note recorded on the arm is superseded and removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
QuantumExplorer
approved these changes
Aug 11, 2026
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v4.2-dev #4364 +/- ##
============================================
- Coverage 87.80% 87.63% -0.18%
============================================
Files 2641 2670 +29
Lines 336510 339447 +2937
============================================
+ Hits 295468 297464 +1996
- Misses 41042 41983 +941
🚀 New features to boost your workflow:
|
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
3 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.
Salvaged from #4240 (closed as superseded by #4284): the shared-Rust deltas that were never part of #4284 — the stale-islock → ChainLock fallback on the L1 invitation claim path, plus the doc recording the IS→CL resubmission arm's test-coverage boundary. Two clean cherry-picks onto current v4.2-dev (authorship preserved); the base's only change to this file since (#4332) does not overlap.
Full review history of the original implementation on #4240.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes