Repository navigation
feat(platform-wallet): own the shielded-tip payment decisions in the wallet #5709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Draft
PastaPastaPasta
wants to merge
21
commits into
feat/dashpay-shielded-tips
Choose a base branch
from
feat/dashpay-shielded-tips-wallet-policy
base: feat/dashpay-shielded-tips
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from 10 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
3dc04f1
feat(platform-wallet): own the shielded-tip payment decisions in the …
PastaPastaPasta 3a21dd5
feat(platform-wallet): parse shielded-tip amounts in the wallet
PastaPastaPasta a73ce5b
fix(platform-wallet): accept keypad decimals and report pre-native ti…
PastaPastaPasta c3ec759
fix(swift-sdk): convert the tip disposition and review constants to U…
PastaPastaPasta 688c75e
fix(platform-wallet): keep a durable barrier per shielded tip until i…
PastaPastaPasta 5e9971b
fix(platform-wallet): trust only local proof that a shielded tip failed
PastaPastaPasta 1309b51
feat(platform-wallet): rebind discovered tip accounts and report tip …
PastaPastaPasta c293c94
fix(platform-wallet): report an unscanned tip account as unavailable,…
PastaPastaPasta 84cfd41
fix(platform-wallet): refuse Clear during a tip send, and never relea…
PastaPastaPasta 0dfe151
fix(platform-wallet): hold identity placement through a tip-balance read
PastaPastaPasta ca92a58
fix(platform-wallet): drain tip sends outside the lifecycle lock, ref…
PastaPastaPasta 9fb9160
fix(platform-wallet): fence tip abandonment against removed wallets, …
PastaPastaPasta ba68897
feat(swift-sdk,kotlin-sdk): refresh shielded accounts off the main ac…
PastaPastaPasta bc99e1e
fix(kotlin-sdk): put isShieldedTipAccount on PlatformWalletManager's …
PastaPastaPasta ffcabad
fix(platform-wallet): refuse dropping accounts while tip barriers are…
PastaPastaPasta 21d3110
fix(platform-wallet): keep an executed tip's notes spent across a sta…
PastaPastaPasta c2b4ca9
fix(platform-wallet): record each observed spend, not a tip's whole i…
PastaPastaPasta 1b30099
fix(platform-wallet): record every owned spend, and keep a recovery r…
PastaPastaPasta 50c24fb
fix(platform-wallet): keep partly confirmed spends bound, and refuse …
PastaPastaPasta 4654d67
fix(platform-wallet): record a spend before marking it, and keep reco…
PastaPastaPasta 158ae6a
fix(platform-wallet): record a spend before settling its tip, scope s…
PastaPastaPasta File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
42 changes: 42 additions & 0 deletions
42
...kotlin-sdk/sdk/src/test/kotlin/org/dashfoundation/dashsdk/tokens/ShieldedTipReviewTest.kt
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,42 @@ | ||
| package org.dashfoundation.dashsdk.tokens | ||
|
|
||
| import org.junit.Assert.assertArrayEquals | ||
| import org.junit.Assert.assertEquals | ||
| import org.junit.Assert.assertFalse | ||
| import org.junit.Assert.assertTrue | ||
| import org.junit.Test | ||
|
|
||
| /** Marshalling of the native tip review and retry decision; the decisions themselves are Rust's. */ | ||
| class ShieldedTipReviewTest { | ||
| private fun review(change: Int, unresolved: Boolean): ByteArray = | ||
| ByteArray(32) { 1 } + ByteArray(43) { 2 } + byteArrayOf(change.toByte()) + | ||
| ByteArray(32) { 3 } + ByteArray(43) { 4 } + byteArrayOf(if (unresolved) 1 else 0) | ||
|
|
||
| @Test | ||
| fun shouldDecodeEachRecipientChange() { | ||
| val new = ShieldedTipReview.fromNative(review(0, unresolved = false)) | ||
| assertEquals(ShieldedTipRecipientChange.New, new.change) | ||
| assertArrayEquals(ByteArray(32) { 1 }, new.recipient.identityId) | ||
| assertArrayEquals(ByteArray(43) { 2 }, new.recipient.address) | ||
| assertFalse(new.unresolved) | ||
|
|
||
| assertEquals( | ||
| ShieldedTipRecipientChange.Unchanged, | ||
| ShieldedTipReview.fromNative(review(1, unresolved = true)).change, | ||
| ) | ||
|
|
||
| val changed = ShieldedTipReview.fromNative(review(2, unresolved = true)) | ||
| val previous = (changed.change as ShieldedTipRecipientChange.Changed).previous | ||
| assertArrayEquals(ByteArray(32) { 3 }, previous.identityId) | ||
| assertArrayEquals(ByteArray(43) { 4 }, previous.address) | ||
| assertTrue(changed.unresolved) | ||
| } | ||
|
|
||
| @Test | ||
| fun shouldTreatAnyDispositionButNotSentAsUnresolved() { | ||
| assertEquals(ShieldedTipDisposition.NOT_SENT, ShieldedTipDisposition.fromNative(1)) | ||
| for (value in listOf(0, 2, -1)) { | ||
| assertEquals(ShieldedTipDisposition.UNRESOLVED, ShieldedTipDisposition.fromNative(value)) | ||
| } | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Suggestion: Carry the native tip disposition through coroutine cancellation
TeardownGate.withOp uses withContext(Dispatchers.IO). Cancelling the caller does not interrupt the blocking native send, and prompt cancellation can discard its completed result when dispatching back to the caller. JNI has already copied Rust's disposition into the array, but this catch throws a bare CancellationException, making cancellation before native entry indistinguishable from cancellation after an executed payment. After success the barrier can already be settled, so another review need not report an unresolved payment either. Preserve the completed native outcome in a cancellation-compatible exception or result without adding a Kotlin retry policy. Add a deterministic completed-send/cancelled-handoff test using the sequence already exercised by TeardownGateTest for discarded secret results.
source:
gpt-6.1-sol(phase2-reviewer: ffi-engineer)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 38bfe13.
PlatformWalletManager.sendShieldedTipruns the gate and the native send underwithContext(NonCancellable). A cancelled caller therefore still gets the completed outcome, and a failure still carries the JNI-written disposition. Cancellation takes effect at the caller's next suspension point.There is no Kotlin test for this. Gradle doesn't run in this environment, and the safety itself doesn't depend on it, because the Rust barrier is what refuses a second payment.
🤖 Posted autonomously by Claude on behalf of pasta.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolved (re-reviewed at
5e9971ba): Your NonCancellable scope now encloses both teardownGate.op and the native send, so the IO handoff returns into a non-cancelled context and preserves the completed outcome and JNI-written disposition. The deterministic cancellation regression remains absent, but the reported result-discard path is addressed.