fix(rust-client): reject zero-amount assets in swap and pswap notes - #2459
Open
Dusk1e wants to merge 1 commit into
Open
fix(rust-client): reject zero-amount assets in swap and pswap notes#2459Dusk1e wants to merge 1 commit into
Dusk1e wants to merge 1 commit into
Conversation
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
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.
Problem
Follow-up to #2457, which closed the same hole on
build_mint_fungible_asset. The remaining note builders still take a zero-amount asset:Both sides of a swap are supposed to carry value.
SwapNote's own docs describe it: the consumer takes the offered asset and "the script creates a P2ID payback note carrying the requested asset" back to the creator.So
requested = 0builds a swap whose payback is a P2ID note carrying nothing - the exact casebuild_pay_to_idrejects withP2IDNoteWithoutAsset, and the one #2457 just closed for minting. Andoffered = 0builds a note whose consumer hands over the requested asset and receives nothing.I should have covered these in #2457; I only looked at the mint path at the time.
Change
Reject a zero-amount asset on either side of
build_swapandbuild_pswap_create.P2IDNoteWithoutAssetdoes not fit here - the offered side is not a P2ID note, and its message and hint are about a P2ID transfer - so this addsSwapNoteWithZeroAsset, carrying which side was zero. It followsZeroExpirationDeltain spirit, and gets anErrorHintlike the neighbouring variants.TransactionRequestErroris not#[non_exhaustive], so the new variant is a breaking change for downstream exhaustive matches; the CHANGELOG records it as such.Swap assets are
Asset, so the zero test goes through a smallis_zero_fungiblehelper - non-fungible assets always carry value. PSWAP is fungible on both sides and compares directly.Test plan
swap_note_with_zero_assetcovers all four cases above. It fails onnext: with the guards removed the firstunwrap_errpanics on anOkrequest.It is a separate test rather than more cases inside
note_without_assetbecause appending there pushed that function pastclippy::too_many_lines.122
miden-clientlib tests still pass. The full unit-test suite runs past 15 minutes locally, so I ran the affected tests and am relying on CI for the rest.