Repository navigation
[ANCHOR-1336]: Record funds-received only for a payment that belongs to the transaction - #2098
Open
ceciliaromao wants to merge 8 commits into
Open
ceciliaromao wants to merge 8 commits into
ceciliaromao wants to merge 8 commits into
Conversation
…ion holds for a transaction
…estination and memo
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.
Description
notify_onchain_funds_receivednow records a ledger hash as the received payment only when the ledger transaction holds a payment to the transaction's receiving account (withdraw_anchor_accountfor SEP-6/24 withdrawals,to_accountfor SEP-31), with the stored memo, in theamount_inasset, for an amount above zero. Otherwise it throwsInvalidParamsExceptionnaming the missing element (destination, memo, asset or amount) and the hash, and changes, saves and publishes nothing. The amount is not compared withamount_expected, so partial payments keep working.notify_onchain_funds_sent(SEP-6/24 deposit payout) applies the same check to the recipient (to_account) and theamount_outasset. The memo is not compared there: payouts do not have to carry the deposit's memo (the reference server's payout carries none).notify_transaction_on_holdno longer setstransferReceivedAt. A hold only changes the status, so an unpaid held transaction can be expired again and its payment can still be recorded.LedgerPaymentMatcher(platform/utils) holds the check: destination, memo, asset, amount, in that order; muxed destinations are matched by their id for any sender, as the payment observer does; a hash memo is accepted in both the hex and base64 forms the observer looks up. The observer itself is unchanged.Context
Testing
./gradlew test./gradlew :platform:test --tests "org.stellar.anchor.platform.utils.LedgerPaymentMatcherTest" --tests "org.stellar.anchor.platform.rpc.NotifyOnchainFundsReceivedHandlerTest" --tests "org.stellar.anchor.platform.rpc.NotifyOnchainFundsSentHandlerTest" --tests "org.stellar.anchor.platform.rpc.NotifyTransactionOnHoldHandlerTest" --tests "org.stellar.anchor.platform.rpc.OnHoldFundsGateTest"New tests cover every rejection cause per protocol, the amount boundary (0 rejected, 1 stroop accepted), several payments in one transaction, muxed destinations, hash memos, an observer-parity test that runs the real
DefaultPaymentListenerand then the RPC, and the on-hold chain../gradlew :essential-tests:test --tests "*Sep24PlatformApiTests*" --tests "*Sep6PlatformApiTests*" --tests "*Sep31PlatformApiTests*"Run locally: SEP-24 9/9, SEP-6 9/10. Not verified locally: the SEP-31 suite (the reference server's creation event arrived about 30 s late and the testnet RPC rejected the observer's polls, so the transaction never reached
pending_senderwithin 60 s) and SEP-6 "deposit complete full with trust flow" (it fails on its first response with the new flow runner switched off too: the reference server completes the deposit before the test's first call). Please rely on CI for these two.The fixtures now pay
destination_accountwith the transaction's memo. The testnet client wallet account (GDJLBYYK…) needs a trustline to Circle'sUSDC:GBBD47IF6LWK7P7MDEVSCWR7DPUWV3NY3DTQEVFL4NAT4AQH3ZLLFLA5for the withdrawal flows to be paid.Documentation
Known limitations
amount_expected(partial payments are intended, see report 3575732).idmemo is forwarded by the observer and refused by the RPC (covered by a test, not changed here).anchor-config-default-values.yaml:119from the report is not touched.