Repository navigation
[ANCHOR-1329]: Fix refund cap check for repeated refund ids in notify_refund_sent and notify_refund_pending - #2097
Open
ceciliaromao wants to merge 7 commits into
Open
ceciliaromao wants to merge 7 commits into
ceciliaromao wants to merge 7 commits 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.
Description
notify_refund_sent(SEP-6 and SEP-24,pending_anchor) now computes its cap and next status over the refund list as it will be stored: the other stored refunds plus the request, with the request's id replacing any stored refund of the same id. Amount and fee are summed at the asset'ssignificant_decimals.notify_refund_pending(SEP-6 and SEP-24 deposits) now replaces a repeatedrefund.idinstead of appending it, and applies the same cap arithmetic.notify_refund_sentfor SEP-31 now rejects a second refund in every supported status with the existing "Multiple refunds aren't supported" error, not only inpending_receiver.NotifyRefundPendingHandlerTest(test_handle_sep24_ok_second_refund,test_handle_sep6_ok_second_refund) asserted the old append behavior (the same id stored twice). They now store a different id for the first refund; the expectations are otherwise unchanged.Context
amount_refunded, while persistence replaced the refund with the same id. A replayed id was therefore checked as x + x but stored as x. The effects were a false flip torefunded, false rejection of a legitimate correction (for example 60 to 70 on anamount_inof 100), and, innotify_refund_pending, a duplicated payment with a doubledamount_refunded.Testing
./gradlew test./gradlew :platform:test --tests "org.stellar.anchor.platform.rpc.RefundIdReplayTest"(new, 142 tests; SEP-6 cases run over deposit, withdrawal, deposit-exchange and withdrawal-exchange)./gradlew :platform:test --tests "org.stellar.anchor.platform.rpc.*"(590 tests, 0 failed)amount_in, new id at and one above the bound, mixed stored fees, 7-decimal amounts and fees, a stored list holding the same id twice,notify_refund_pendingreplacing a repeated id, the pending / sent / pending sequence, rejected calls leaving the transaction unchanged, and SEP-31 inpending_stellarandpending_receiver.notify_refund_sentwith the samerefund.idand amount repeatedly, andnotify_refund_pendingtwice with the same id (amount 0.4, fee 0.01 onamount_in1).amount_refundedand the payments list should match one refund.Documentation
Known limitations
pending_stellartoo. No RPC route reaches that state with a stored refund, so this only matters for transactions changed throughPATCH /transactions.