From 33e5a6fa7cafd2629169bb9e01ef3421791c6dfd Mon Sep 17 00:00:00 2001 From: Chenna Keshava B S <21219765+ckeshava@users.noreply.github.com> Date: Tue, 13 Oct 2026 19:07:45 +0000 Subject: [PATCH] feat: Add TransactionProposalCancel & Cosign Ticket reservation (#8538) Co-authored-by: Shawn Xie <35279399+shawnxie999@users.noreply.github.com> Co-authored-by: Kassaking7 <96991820+Kassaking7@users.noreply.github.com> Co-authored-by: Shawn Xie Co-authored-by: Zhiyuan Wang <1830604455@qq.com> Co-authored-by: Claude Fable 5 --- include/xrpl/ledger/helpers/ProposalHelpers.h | 129 +- include/xrpl/protocol/TER.h | 2 + .../xrpl/protocol/detail/transactions.macro | 14 + .../transactions/TransactionProposalCancel.h | 131 ++ include/xrpl/tx/Transactor.h | 16 + .../proposal/TransactionProposalCancel.h | 44 + .../ledger/helpers/ProposalHelpers.cpp | 162 +- src/libxrpl/protocol/TER.cpp | 1 + src/libxrpl/tx/Transactor.cpp | 37 +- .../proposal/TransactionProposalCancel.cpp | 119 ++ .../proposal/TransactionProposalCreate.cpp | 2 +- .../app/TransactionProposalCancel_test.cpp | 1356 +++++++++++++ .../app/TransactionProposalCreate_test.cpp | 49 +- ...TransactionProposalReservedTicket_test.cpp | 1775 +++++++++++++++++ src/test/jtx/impl/proposal.cpp | 10 + src/test/jtx/proposal.h | 11 + .../TransactionProposalCancelTests.cpp | 146 ++ src/tests/libxrpl/tx/ProposalHelpers.cpp | 25 +- 18 files changed, 3987 insertions(+), 42 deletions(-) create mode 100644 include/xrpl/protocol_autogen/transactions/TransactionProposalCancel.h create mode 100644 include/xrpl/tx/transactors/proposal/TransactionProposalCancel.h create mode 100644 src/libxrpl/tx/transactors/proposal/TransactionProposalCancel.cpp create mode 100644 src/test/app/TransactionProposalCancel_test.cpp create mode 100644 src/test/app/TransactionProposalReservedTicket_test.cpp create mode 100644 src/tests/libxrpl/protocol_autogen/transactions/TransactionProposalCancelTests.cpp diff --git a/include/xrpl/ledger/helpers/ProposalHelpers.h b/include/xrpl/ledger/helpers/ProposalHelpers.h index 0a09dfd887..66141be3fc 100644 --- a/include/xrpl/ledger/helpers/ProposalHelpers.h +++ b/include/xrpl/ledger/helpers/ProposalHelpers.h @@ -1,10 +1,17 @@ #pragma once +#include +#include +#include #include +#include #include +#include +#include #include #include +#include namespace xrpl::proposal { @@ -30,31 +37,22 @@ proposalOwnerCount(STObject const& proposedTx) : kProposalOwnerCount; } -/** - * Whether the proposed transaction is itself a proposal transaction, which - * would nest one proposal inside another. - * - * TODO: cover ttTRANSACTION_PROPOSAL_SIGN and ttTRANSACTION_PROPOSAL_CANCEL - * once those transactions exist. - */ -inline bool -isProposalTx(STObject const& proposedTx) -{ - return proposedTx.getFieldU16(sfTransactionType) == ttTRANSACTION_PROPOSAL_CREATE; -} - /** * Whether the proposed transaction is independently submittable through the - * ordinary multi-sign path: not a nested proposal, not a pseudo-transaction, - * not itself flagged as someone else's inner batch transaction, and — if it - * is a Batch — none of its own inner transactions is a nested proposal or a - * pseudo-transaction either. A Batch inner transaction cannot itself be - * pseudo (preflight0 rejects the pseudo/tfInnerBatchTxn combination - * generically), but that guard lives outside this feature, so it is checked - * again here rather than relied upon. + * ordinary multi-sign path: not a proposal transaction itself, not a + * pseudo-transaction, not itself flagged as someone else's inner batch + * transaction, and — if it is a Batch — every one of its own inner + * transactions carries a TransactionType and is neither a proposal, a + * pseudo-transaction, nor a nested Batch. A Batch inner transaction cannot + * lack a TransactionType, be pseudo, or be a nested Batch (STTx construction + * rejects a typeless inner and an inner Batch, and preflight0 rejects the + * pseudo/tfInnerBatchTxn combination), but those guards live outside this + * feature, so they are checked again here rather than relied upon. + * + * TODO: cover ttTRANSACTION_PROPOSAL_SIGN once that transaction exists. */ bool -isValidProposal(STObject const& proposedTx); +isValidProposalTxnType(STObject const& proposedTx); /** * Whether the proposed transaction carries any signature field. @@ -84,4 +82,93 @@ hasEmptySigningPubKey(STObject const& proposedTx) proposedTx.getFieldVL(sfSigningPubKey).empty(); } +/** + * Whether the proposal is terminal (On-Chain Cosigner spec §4.5). A terminal proposal + * stops accepting signatures and anyone may delete it. An already signed + * payload can still execute after Expiration, because the Ticket reservation + * compares payloads only; a passed LastLedgerSequence does stop it. + * + * A proposal is terminal when either: + * - its Expiration has passed (the parent ledger closed at or after it), or + * - the proposed transaction carries a LastLedgerSequence and the current + * ledger sequence is strictly greater than it. This mirrors the ordinary + * transactor's tefMAX_LEDGER rule: at view.seq() == LastLedgerSequence the + * payload can still be submitted in the current ledger, so the proposal is + * still live. + * + * @param view The ledger the deciding transaction is being applied to. + * @param expiration The proposal's Expiration field. + * @param proposedTx The proposal's ProposedTransaction field. + */ +bool +isTerminal( + ReadView const& view, + std::optional expiration, + STObject const& proposedTx); + +/** + * Delete a TransactionProposal ledger entry. + * + * Removes the entry from its Owner's directory, releases the reserve the + * proposal holds against the Owner, and erases the entry. Shared by every + * deletion path the spec defines (On-Chain Cosigner spec §4.5): automatic cleanup when the + * proposed transaction's TicketSequence is consumed, and the + * TransactionProposalCancel / TransactionProposalSign cleanup paths once + * those transactions exist. + * + * The release goes through decreaseOwnerCountForObject, so it follows the + * sfSponsor an entry created with reserve sponsorship carries: the reserve is + * returned to whoever was holding it, the Owner or that sponsor. + * + * @param view The apply view for making changes + * @param sleProposal The TransactionProposal ledger entry to delete + * @param j Journal for logging + * @return tesSUCCESS, tecINTERNAL for an invalid entry, or tefBAD_LEDGER if + * the ledger contradicts the entry + */ +TER +deleteProposal(ApplyView& view, SLE::pointer const& sleProposal, beast::Journal j); + +/** + * Whether tx carries the same payload as a proposal's stored + * ProposedTransaction (On-Chain Cosigner spec §4.2.1). + * + * The payload is the signing payload: the fields the collected signatures + * commit to. Compared is every field except those flagged kNotSigning + * (TxnSignature, Signers, BatchSigners, CounterpartySignature, + * SponsorSignature, Signature, MasterSignature) and SigningPubKey, which is + * stored empty and filled only if the target signs with its own key. So the + * completed transaction matches no matter which mix of collected or + * off-ledger signatures it carries, and any change to a non-signature field + * results in a false-return value. + * + * Signature (PaymentChannelClaim and the XChain attestation transactions) + * is deliberately not compared: it is outside the account's own TxnSignature + * even for an ordinary submission, so a proposal cannot bind it more tightly + * than the signatures it collects do, and those transactors still validate a + * substituted value exactly as they would for an ordinary submission. + * MasterSignature appears in no transaction format. + * + * @param proposedTx The proposal's ProposedTransaction field. + * @param tx The transaction to compare, typically an STTx. + */ +bool +payloadMatches(STObject const& proposedTx, STObject const& tx); + +/** + * Whether tx may consume its Ticket. + * + * While a proposal keyed to the Ticket exists, the target's Ticket is + * reserved: only the proposal's own proposed transaction may spend it + * (On-Chain Cosigner spec §4.2.1), so that unrelated target-account activity cannot + * invalidate the proposal while signatures are being collected. + * TransactionProposalCancel is not allowed through: it deletes the + * reservation without consuming the ticket. + * + * @param view The ledger containing the Ticket and any reservation. + * @param tx The transaction attempting to consume that ticket. + */ +bool +canConsumeTicket(ReadView const& view, STTx const& tx); + } // namespace xrpl::proposal diff --git a/include/xrpl/protocol/TER.h b/include/xrpl/protocol/TER.h index 7f7eb9b7d5..69f454ff82 100644 --- a/include/xrpl/protocol/TER.h +++ b/include/xrpl/protocol/TER.h @@ -231,6 +231,8 @@ enum TERcodes : TERUnderlyingType { terNO_DELEGATE_PERMISSION, // Delegate does not have permission terLOCKED, // MPT is locked terNO_PERMISSION, // No permission but retry + terTICKET_RESERVED, // Ticket is reserved and cannot be consumed + // until the reservation is released }; //------------------------------------------------------------------------------ diff --git a/include/xrpl/protocol/detail/transactions.macro b/include/xrpl/protocol/detail/transactions.macro index 084a536840..42e8adff1a 100644 --- a/include/xrpl/protocol/detail/transactions.macro +++ b/include/xrpl/protocol/detail/transactions.macro @@ -1175,6 +1175,20 @@ TRANSACTION(ttTRANSACTION_PROPOSAL_CREATE, 95, TransactionProposalCreate, {sfExpiration, SoeRequired}, })) +// ttTRANSACTION_PROPOSAL_SIGN arrives in a later PR and takes the next unassigned type. + +/** This transaction deletes a TransactionProposal, releasing its owner's + reserve. While the proposal is live only its Owner or its target account + may cancel; once it is terminal anyone may (On-Chain Cosigner spec §7). */ +#if TRANSACTION_INCLUDE +# include +#endif +TRANSACTION(ttTRANSACTION_PROPOSAL_CANCEL, 94, TransactionProposalCancel, + ({.amendment = featureCosign}), + ({ + {sfProposalID, SoeRequired}, +})) + /** This system-generated transaction type is used to update the status of the various amendments. For details, see: https://xrpl.org/amendments.html diff --git a/include/xrpl/protocol_autogen/transactions/TransactionProposalCancel.h b/include/xrpl/protocol_autogen/transactions/TransactionProposalCancel.h new file mode 100644 index 0000000000..b69f45f876 --- /dev/null +++ b/include/xrpl/protocol_autogen/transactions/TransactionProposalCancel.h @@ -0,0 +1,131 @@ +// This file is auto-generated. Do not edit. +#pragma once + +#include +#include +#include +#include +#include +#include + +#include +#include + +namespace xrpl::transactions { + +class TransactionProposalCancelBuilder; + +/** + * @brief Transaction: TransactionProposalCancel + * + * Type: ttTRANSACTION_PROPOSAL_CANCEL (94) + * Delegable: Delegation::NotDelegable + * Amendment: featureCosign + * Privileges: Privilege::NoPriv + * + * Immutable wrapper around STTx providing type-safe field access. + * Use TransactionProposalCancelBuilder to construct new transactions. + */ +class TransactionProposalCancel : public TransactionBase +{ +public: + static constexpr xrpl::TxType txType = ttTRANSACTION_PROPOSAL_CANCEL; + + /** + * @brief Construct a TransactionProposalCancel transaction wrapper from an existing STTx object. + * @throws std::runtime_error if the transaction type doesn't match. + */ + explicit TransactionProposalCancel(std::shared_ptr tx) + : TransactionBase(std::move(tx)) + { + // Verify transaction type + if (tx_->getTxnType() != txType) + { + throw std::runtime_error("Invalid transaction type for TransactionProposalCancel"); + } + } + + // Transaction-specific field getters + + /** + * @brief Get sfProposalID (SoeRequired) + * @return The field value. + */ + [[nodiscard]] + SF_UINT256::type::value_type + getProposalID() const + { + return this->tx_->at(sfProposalID); + } +}; + +/** + * @brief Builder for TransactionProposalCancel transactions. + * + * Provides a fluent interface for constructing transactions with method chaining. + * Uses STObject internally for flexible transaction construction. + * Inherits common field setters from TransactionBuilderBase. + */ +class TransactionProposalCancelBuilder : public TransactionBuilderBase +{ +public: + /** + * @brief Construct a new TransactionProposalCancelBuilder with required fields. + * @param account The account initiating the transaction. + * @param proposalID The sfProposalID field value. + * @param sequence Optional sequence number for the transaction. + * @param fee Optional fee for the transaction. + */ + TransactionProposalCancelBuilder(SF_ACCOUNT::type::value_type account, + std::decay_t const& proposalID, std::optional sequence = std::nullopt, + std::optional fee = std::nullopt +) + : TransactionBuilderBase(ttTRANSACTION_PROPOSAL_CANCEL, account, sequence, fee) + { + setProposalID(proposalID); + } + + /** + * @brief Construct a TransactionProposalCancelBuilder from an existing STTx object. + * @param tx The existing transaction to copy from. + * @throws std::runtime_error if the transaction type doesn't match. + */ + TransactionProposalCancelBuilder(std::shared_ptr tx) + { + if (tx->getTxnType() != ttTRANSACTION_PROPOSAL_CANCEL) + { + throw std::runtime_error("Invalid transaction type for TransactionProposalCancelBuilder"); + } + object_ = *tx; + } + + /** + * @brief Transaction-specific field setters + */ + + /** + * @brief Set sfProposalID (SoeRequired) + * @return Reference to this builder for method chaining. + */ + TransactionProposalCancelBuilder& + setProposalID(std::decay_t const& value) + { + object_[sfProposalID] = value; + return *this; + } + + /** + * @brief Build and return the TransactionProposalCancel wrapper. + * @param publicKey The public key for signing. + * @param secretKey The secret key for signing. + * @return The constructed transaction wrapper. + */ + TransactionProposalCancel + build(PublicKey const& publicKey, SecretKey const& secretKey) + { + sign(publicKey, secretKey); + return TransactionProposalCancel{std::make_shared(std::move(object_))}; + } +}; + +} // namespace xrpl::transactions diff --git a/include/xrpl/tx/Transactor.h b/include/xrpl/tx/Transactor.h index c060836eec..2e275cd8dd 100644 --- a/include/xrpl/tx/Transactor.h +++ b/include/xrpl/tx/Transactor.h @@ -375,6 +375,22 @@ public: ///////////////////////////////////////////////////// // Interface used by AccountDelete + /** + * This method removes one Ticket ledger-object from the ledger. AccountDelete uses it to + * clean up Ticket objects en masse. + * + * This method reduces the TicketCount and the OwnerCount of the Owner AccountRoot object. If + * a TransactionProposal reserved this Ticket, that proposal is deleted as well: once the + * Ticket is gone, its proposed transaction can never be applied (tefNO_TICKET). + * + * @param view the apply view for making changes. + * @param account the account which owns the Ticket object. + * @param ticketIndex ticketIndex is used to construct the ledger-entry ID of Ticket object. + * @param j the journal reference used to log messages. + * + * @return the final TER after all invariant checks. `tefBAD_LEDGER` is returned if Ticket, + * Owner AccountRoot is not found in the apply-view ledger. + */ static TER ticketDelete( ApplyView& view, diff --git a/include/xrpl/tx/transactors/proposal/TransactionProposalCancel.h b/include/xrpl/tx/transactors/proposal/TransactionProposalCancel.h new file mode 100644 index 0000000000..88e904497b --- /dev/null +++ b/include/xrpl/tx/transactors/proposal/TransactionProposalCancel.h @@ -0,0 +1,44 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include +#include + +namespace xrpl { + +class TransactionProposalCancel : public Transactor +{ +public: + static constexpr auto kConsequencesFactory = ConsequencesFactoryType::Normal; + + explicit TransactionProposalCancel(ApplyContext& ctx) : Transactor(ctx) + { + } + + static NotTEC + preflight(PreflightContext const& ctx); + + static TER + preclaim(PreclaimContext const& ctx); + + TER + doApply() override; + + void + visitInvariantEntry(bool isDelete, SLE::ConstRef before, SLE::ConstRef after) override; + + [[nodiscard]] bool + finalizeInvariants( + STTx const& tx, + TER result, + XRPAmount fee, + ReadView const& view, + beast::Journal const& j) override; +}; + +} // namespace xrpl diff --git a/src/libxrpl/ledger/helpers/ProposalHelpers.cpp b/src/libxrpl/ledger/helpers/ProposalHelpers.cpp index 4adf57ab3a..876a3872f0 100644 --- a/src/libxrpl/ledger/helpers/ProposalHelpers.cpp +++ b/src/libxrpl/ledger/helpers/ProposalHelpers.cpp @@ -1,18 +1,51 @@ #include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include #include #include +#include #include #include +#include +#include #include #include +#include +#include +#include +#include + namespace xrpl::proposal { +namespace { + +// Proposals do not nest, so a proposed transaction may carry none of these +// types, neither as its own payload nor as a Batch inner transaction. +constexpr auto kProposalTxTypes = std::to_array({ + ttTRANSACTION_PROPOSAL_CREATE, + ttTRANSACTION_PROPOSAL_CANCEL, +}); + +} // namespace + bool -isValidProposal(STObject const& proposedTx) +isValidProposalTxnType(STObject const& proposedTx) { - if (isProposalTx(proposedTx)) + auto const type = proposedTx.getFieldU16(sfTransactionType); + + if (std::ranges::find(kProposalTxTypes, type) != kProposalTxTypes.end()) return false; if (isPseudoTx(proposedTx)) @@ -22,13 +55,24 @@ isValidProposal(STObject const& proposedTx) (proposedTx.getFieldU32(sfFlags) & tfInnerBatchTxn) != 0u) return false; - if (proposedTx.getFieldU16(sfTransactionType) == ttBATCH && - proposedTx.isFieldPresent(sfRawTransactions)) + if (type == ttBATCH && proposedTx.isFieldPresent(sfRawTransactions)) { STArray const& innerTxns = proposedTx.getFieldArray(sfRawTransactions); for (STObject const& inner : innerTxns) { - if (isProposalTx(inner) || isPseudoTx(inner)) + // The only production caller reaches this walk through STTx + // construction, which rejects a typeless inner first, so this is + // re-checked rather than relied upon. + if (!inner.isFieldPresent(sfTransactionType)) + return false; + + auto const innerType = inner.getFieldU16(sfTransactionType); + + // A nested Batch is rejected alongside nested proposals and + // pseudo-transactions: STTx construction forbids inner Batches, so + // a proposal must not be able to store one either. + if (std::ranges::find(kProposalTxTypes, innerType) != kProposalTxTypes.end() || + innerType == ttBATCH || isPseudoTx(inner)) return false; } } @@ -36,4 +80,112 @@ isValidProposal(STObject const& proposedTx) return true; } +bool +isTerminal( + ReadView const& view, + std::optional expiration, + STObject const& proposedTx) +{ + if (hasExpired(view, expiration)) + return true; + + // Mirror the ordinary transactor's tefMAX_LEDGER rule: view.seq() > bound. + // At view.seq() == bound the payload can still be submitted, so the + // proposal is still live (On-Chain Cosigner spec §4.5). + return proposedTx.isFieldPresent(sfLastLedgerSequence) && + view.seq() > proposedTx.getFieldU32(sfLastLedgerSequence); +} + +TER +deleteProposal(ApplyView& view, SLE::pointer const& sleProposal, beast::Journal j) +{ + if (!sleProposal || sleProposal->getType() != ltTRANSACTION_PROPOSAL || + !view.exists(Keylet{ltTRANSACTION_PROPOSAL, sleProposal->key()})) + { + // LCOV_EXCL_START + JLOG(j.fatal()) << "Invalid TransactionProposal deletion."; + return tecINTERNAL; + // LCOV_EXCL_STOP + } + + AccountID const owner = sleProposal->getAccountID(sfOwner); + + std::uint64_t const page{(*sleProposal)[sfOwnerNode]}; + if (!view.dirRemove(keylet::ownerDir(owner), page, sleProposal->key(), true)) + { + // LCOV_EXCL_START + JLOG(j.fatal()) << "Unable to delete TransactionProposal from owner."; + return tefBAD_LEDGER; + // LCOV_EXCL_STOP + } + + auto const sleOwner = view.peek(keylet::account(owner)); + if (!sleOwner) + { + // LCOV_EXCL_START + JLOG(j.fatal()) << "Could not find TransactionProposal owner account root."; + return tefBAD_LEDGER; + // LCOV_EXCL_STOP + } + + // Release the reserve against the Owner or, if the entry carries a + // reserve sponsor, against that sponsor. + decreaseOwnerCountForObject( + view, + sleOwner, + sleProposal, + proposalOwnerCount(sleProposal->getFieldObject(sfProposedTransaction)), + j); + + view.erase(sleProposal); + return tesSUCCESS; +} + +bool +payloadMatches(STObject const& proposedTx, STObject const& tx) +{ + XRPL_ASSERT( + proposedTx.isFieldPresent(sfTransactionType) && tx.isFieldPresent(sfTransactionType), + "xrpl::proposal::payloadMatches : transaction-shaped inputs"); + + // The signing-payload serialization omits every kNotSigning field: the + // signature containers a proposal lets evolve (TxnSignature, Signers, + // BatchSigners, CounterpartySignature, SponsorSignature) plus Signature + // and MasterSignature, which the account's own signature does not cover + // either (see the header). What remains is the field set fixed at + // creation — except SigningPubKey, which signing payloads include but + // which is also mutable here (stored empty, filled if the target signs + // with its own key). Neutralize it on both sides before comparing. + auto const payloadBytes = [](STObject const& obj) { + STObject copy{obj}; + copy.setFieldVL(sfSigningPubKey, Slice{}); + Serializer s; + copy.addWithoutSigningFields(s); + return s.getData(); + }; + + return payloadBytes(proposedTx) == payloadBytes(tx); +} + +bool +canConsumeTicket(ReadView const& view, STTx const& tx) +{ + auto const seqProx = tx.getSeqProxy(); + if (!seqProx.isTicket()) + { + // The caller only reaches here for a Ticket-based transaction. + // LCOV_EXCL_START + UNREACHABLE("xrpl::proposal::canConsumeTicket : tx spends a Sequence"); + return false; + // LCOV_EXCL_STOP + } + + auto const sleProposal = + view.read(keylet::txProposal(tx.getAccountID(sfAccount), seqProx.value())); + if (!sleProposal) + return true; + + return payloadMatches(sleProposal->getFieldObject(sfProposedTransaction), tx); +} + } // namespace xrpl::proposal diff --git a/src/libxrpl/protocol/TER.cpp b/src/libxrpl/protocol/TER.cpp index 6a15ebdba5..889a558993 100644 --- a/src/libxrpl/protocol/TER.cpp +++ b/src/libxrpl/protocol/TER.cpp @@ -228,6 +228,7 @@ transResults() MAKE_ERROR(terNO_DELEGATE_PERMISSION, "Delegated account lacks permission to perform this transaction."), MAKE_ERROR(terLOCKED, "Fund is locked."), MAKE_ERROR(terNO_PERMISSION, "No permission to perform requested operation."), + MAKE_ERROR(terTICKET_RESERVED, "Ticket is reserved."), MAKE_ERROR(tesSUCCESS, "The transaction was applied. Only final in a validated ledger."), }; diff --git a/src/libxrpl/tx/Transactor.cpp b/src/libxrpl/tx/Transactor.cpp index 62d6624518..69efc92029 100644 --- a/src/libxrpl/tx/Transactor.cpp +++ b/src/libxrpl/tx/Transactor.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -327,10 +328,9 @@ Transactor::preflight2(PreflightContext const& ctx) auto const sigValid = checkValidity(ctx.registry.get().getHashRouter(), ctx.tx, ctx.rules); if (sigValid.first == Validity::SigBad) - { // LCOV_EXCL_START + { JLOG(ctx.j.debug()) << "preflight2: bad signature. " << sigValid.second; return temINVALID; - // LCOV_EXCL_STOP } // Do not add any checks after this point that are relevant for @@ -755,6 +755,19 @@ Transactor::checkSeqProxy(ReadView const& view, STTx const& tx, beast::Journal j << "a_seq=" << aSeq << " t_seq=" << tSeqProx; return tefNO_TICKET; } + + // While a TransactionProposal keyed to this Ticket exists, the + // Ticket is reserved: only the proposal's own proposed transaction + // may consume it, so unrelated activity cannot invalidate the + // proposal while signatures are being collected (On-Chain Cosigner spec §4.2.1). + // Cancelling the proposal frees the Ticket, so this is a retry, not + // a final failure. No proposal can exist before the Cosign amendment, + // so the gate only makes that dependency explicit. + if (view.rules().enabled(featureCosign) && !proposal::canConsumeTicket(view, tx)) + { + JLOG(j.trace()) << "applyTransaction: ticket " << tSeqProx << " is reserved"; + return terTICKET_RESERVED; + } } return tesSUCCESS; @@ -866,8 +879,28 @@ Transactor::ticketDelete( // Update the Ticket owner's reserve. decreaseOwnerCountForObject(view, sleAccount, sleTicket, 1, j); + std::uint32_t const ticketSeq{(*sleTicket)[sfTicketSequence]}; + // Remove Ticket from ledger. view.erase(sleTicket); + + // Once the Ticket is gone, a TransactionProposal keyed to it can never + // execute: its proposed transaction would fail with tefNO_TICKET. Clean + // up the stale proposal and release its Owner's reserve (On-Chain Cosigner spec §4.5). + // The reservation check in checkSeqProxy means this is reached only by + // the proposal's own proposed transaction executing (or failing with a + // claimed-fee tec), or by AccountDelete sweeping the target account's + // Tickets. No proposal can exist before the Cosign amendment, so the gate + // only makes that dependency explicit. + if (view.rules().enabled(featureCosign)) + { + if (auto const sleProposal = view.peek(keylet::txProposal(account, ticketSeq))) + { + if (TER const ter = proposal::deleteProposal(view, sleProposal, j); !isTesSuccess(ter)) + return ter; // LCOV_EXCL_LINE + } + } + return tesSUCCESS; } diff --git a/src/libxrpl/tx/transactors/proposal/TransactionProposalCancel.cpp b/src/libxrpl/tx/transactors/proposal/TransactionProposalCancel.cpp new file mode 100644 index 0000000000..b7306101f8 --- /dev/null +++ b/src/libxrpl/tx/transactors/proposal/TransactionProposalCancel.cpp @@ -0,0 +1,119 @@ +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +namespace xrpl { + +NotTEC +TransactionProposalCancel::preflight(PreflightContext const& ctx) +{ + if (ctx.tx[sfProposalID] == beast::kZero) + { + JLOG(ctx.j.debug()) << "TransactionProposalCancel: zero ProposalID."; + return temMALFORMED; + } + + return tesSUCCESS; +} + +TER +TransactionProposalCancel::preclaim(PreclaimContext const& ctx) +{ + uint256 const proposalID = ctx.tx[sfProposalID]; + + // Never read the zero keylet. Preflight already rejected a zero + // ProposalID, and no real proposal can have one (every ProposalID is a + // hash), so reaching here with zero is an internal error. (and a mathematical rarity. ) + if (proposalID == beast::kZero) + return tefINTERNAL; // LCOV_EXCL_LINE + + auto const sleProposal = ctx.view.read(keylet::txProposal(proposalID)); + if (!sleProposal) + { + JLOG(ctx.j.debug()) << "TransactionProposalCancel: proposal does not exist."; + return tecNO_ENTRY; + } + + STObject const proposedTx = sleProposal->getFieldObject(sfProposedTransaction); + + // A terminal proposal is closed to new signatures, so anyone may delete + // it and release the Owner's reserve (On-Chain Cosigner spec §7.2). Being + // terminal does not stop an already signed payload from executing; see + // proposal::isTerminal. + bool const terminal = proposal::isTerminal(ctx.view, (*sleProposal)[~sfExpiration], proposedTx); + + // A live proposal may only be cancelled by its Owner (the proposer) or + // by its target. The target is the account the proposed transaction + // would execute against (its Account field) or the Delegate named in it. + // Targets may always refuse a proposal, because anyone can create one + // against any account and doing so ties up one of that account's + // tickets (On-Chain Cosigner spec §7.2). + // + // The Delegate is read from the stored payload as-is; whether that + // delegation exists on-ledger is not checked. The proposal asks that + // account to sign, so that account may refuse — and refusing only + // deletes the proposer's own entry and returns the proposer's own + // reserve. + AccountID const account = ctx.tx[sfAccount]; + bool const ownerOrTarget = account == sleProposal->getAccountID(sfOwner) || + account == proposedTx.getAccountID(sfAccount) || account == proposedTx[~sfDelegate]; + + if (!terminal && !ownerOrTarget) + { + JLOG(ctx.j.debug()) << "TransactionProposalCancel: proposal is live and " + "canceller is neither its Owner nor its target."; + return tecNO_PERMISSION; + } + + return tesSUCCESS; +} + +TER +TransactionProposalCancel::doApply() +{ + uint256 const proposalID = ctx_.tx[sfProposalID]; + + // Never read the zero keylet; see the matching guard in preclaim. + if (proposalID == beast::kZero) + return tefINTERNAL; // LCOV_EXCL_LINE + + auto const sleProposal = view().peek(keylet::txProposal(proposalID)); + if (!sleProposal) + return tefINTERNAL; // LCOV_EXCL_LINE + + return proposal::deleteProposal(view(), sleProposal, ctx_.registry.get().getJournal("View")); +} + +void +TransactionProposalCancel::visitInvariantEntry(bool, SLE::ConstRef, SLE::ConstRef) +{ + // No transaction-specific invariants yet (future work). +} + +bool +TransactionProposalCancel::finalizeInvariants( + STTx const&, + TER, + XRPAmount, + ReadView const&, + beast::Journal const&) +{ + // No transaction-specific invariants yet (future work). + return true; +} + +} // namespace xrpl diff --git a/src/libxrpl/tx/transactors/proposal/TransactionProposalCreate.cpp b/src/libxrpl/tx/transactors/proposal/TransactionProposalCreate.cpp index 46818490a5..cd83e0da01 100644 --- a/src/libxrpl/tx/transactors/proposal/TransactionProposalCreate.cpp +++ b/src/libxrpl/tx/transactors/proposal/TransactionProposalCreate.cpp @@ -84,7 +84,7 @@ TransactionProposalCreate::preflight(PreflightContext const& ctx) // no batch inner transactions — and, if it is a Batch, none of its own // inner transactions may be a nested proposal or a pseudo-transaction // either. - if (!proposal::isValidProposal(proposedTx)) + if (!proposal::isValidProposalTxnType(proposedTx)) { JLOG(ctx.j.debug()) << "TransactionProposalCreate: proposed txn is not " "independently submittable."; diff --git a/src/test/app/TransactionProposalCancel_test.cpp b/src/test/app/TransactionProposalCancel_test.cpp new file mode 100644 index 0000000000..3d9991908c --- /dev/null +++ b/src/test/app/TransactionProposalCancel_test.cpp @@ -0,0 +1,1356 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include + +namespace xrpl::test { + +struct TransactionProposalCancel_test : public beast::unit_test::Suite +{ + // Create one ticket for the target and a live proposal keyed to it, + // returning the proposal's ID. The proposal is the proposer's and its + // proposed transaction pays dest. + uint256 + makeProposal( + jtx::Env& env, + jtx::Account const& proposer, + jtx::Account const& target, + jtx::Account const& dest, + std::optional payloadOverride = std::nullopt) + { + using namespace jtx; + using namespace std::chrono_literals; + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + json::Value const payload = payloadOverride + ? *payloadOverride + : proposal::unsignedPayload(env, pay(target, dest, XRP(1)), ticketSeq); + + if (proposer.id() != target.id() && !env.le(keylet::signerList(target.id()))) + proposal::authorizeProposer(env, target, proposer); + + env(proposal::create(proposer, payload, proposal::expiration(env, 1000s))); + env.close(); + + uint256 const proposalID = keylet::txProposal(target.id(), ticketSeq).key; + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + return proposalID; + } + + // Nothing about the transaction is available before the amendment is + // active. + void + testDisabled(FeatureBitset features) + { + testcase("amendment disabled"); + + using namespace jtx; + + Env env{*this, features - featureCosign}; + + Account const alice{"alice"}; + env.fund(XRP(10000), alice); + env.close(); + + env(proposal::cancel(alice, uint256{1}), Ter(temDISABLED)); + env.close(); + } + + // Static rejections: a zero ProposalID identifies nothing, and the + // transaction defines no flags of its own. + void + testMalformed(FeatureBitset features) + { + testcase("malformed transaction"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + env.fund(XRP(10000), alice); + env.close(); + + env(proposal::cancel(alice, uint256{}), Ter(temMALFORMED)); + env.close(); + + env(proposal::cancel(alice, uint256{1}), Txflags(0x00010000), Ter(temINVALID_FLAG)); + env.close(); + } + + // The named proposal must exist as a TransactionProposal entry: + // (1) an ID that matches no object + // (2) an entry of a different type + // (3) A previously cancelled ProposalID + // All the above cases will result in tecNO_ENTRY + void + testNoEntry(FeatureBitset features) + { + testcase("proposal does not exist"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + env.fund(XRP(10000), alice, target); + env.close(); + proposal::authorizeProposer(env, target, alice); + + // No such entry at all. + env(proposal::cancel(alice, uint256{1}), Ter(tecNO_ENTRY)); + env.close(); + + // The ID of a live ledger entry of a different type (the ticket the + // proposal below will consume) does not name a proposal. + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + env(proposal::cancel( + alice, keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)).key), + Ter(tecNO_ENTRY)); + env.close(); + + // A proposal that was already cancelled is gone: a second cancel — + // even by its Owner — finds nothing. + env(proposal::create( + alice, + proposal::unsignedPayload(env, pay(target, alice, XRP(1)), ticketSeq), + proposal::expiration(env, 1000s))); + env.close(); + uint256 const proposalID = keylet::txProposal(target.id(), ticketSeq).key; + + env(proposal::cancel(alice, proposalID)); + env.close(); + env(proposal::cancel(alice, proposalID), Ter(tecNO_ENTRY)); + env.close(); + } + + // The Owner may cancel a live proposal. Cancellation deletes the entry, + // releases the Owner's reserve, and frees the (target, ticket) slot — but + // leaves the target's Ticket itself intact and usable. + void + testOwnerCancel(FeatureBitset features) + { + testcase("owner cancels live proposal"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + + std::uint32_t const ticketSeq = env.seq(target) + 1; + uint256 const proposalID = makeProposal(env, alice, target, bob); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + + env(proposal::cancel(alice, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // The ticket the proposed transaction would have consumed still + // belongs to the target. + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, target) == 2); + + // The (target, ticket) slot is free again: a new proposal for the + // same pair is not a duplicate. + env(proposal::create( + alice, + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq), + proposal::expiration(env, 1000s))); + env.close(); + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + + // And once cancelled again, the target may spend the ticket on + // something else entirely (which is also what positively revokes a + // proposal: cancellation alone does not invalidate signatures an + // observer may have copied, On-Chain Cosigner spec §13.4). + env(proposal::cancel(alice, proposalID)); + env.close(); + env(noop(target), ticket::Use(ticketSeq)); + env.close(); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + } + + // The target account may refuse any proposal made for it, at any point in + // its lifecycle, without owning the object: anyone can create a proposal + // against any account and tie up one of its tickets (On-Chain Cosigner spec §7.2). + void + testTargetCancel(FeatureBitset features) + { + testcase("target cancels live proposal"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + + uint256 const proposalID = makeProposal(env, alice, target, bob); + + env(proposal::cancel(target, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + // The reserve was the proposer's, and cancellation returns it to the + // proposer even when someone else cancels. + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // The target account still holds the SignerList plus one unused ticket. + BEAST_EXPECT(ownerCount(env, target) == 2); + } + + // An account may propose against itself, collapsing the Owner and the + // target into one account. Two things follow that no other case in this + // suite reaches. Create skips its authorization check entirely for a + // self-proposal, so no SignerList is involved anywhere; and the account + // holding the reserve is also the account being granted permission, so the + // release has to happen once rather than once per satisfied arm of that + // test — the owner count is the only thing that would show the difference. + void + testSelfProposalCancel(FeatureBitset features) + { + testcase("target cancels a proposal it made for itself"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const target{"target"}; // both the proposer and the target + Account const bob{"bob"}; + env.fund(XRP(10000), target, bob); + env.close(); + + std::uint32_t const ticketSeq = env.seq(target) + 1; + uint256 const proposalID = makeProposal(env, target, target, bob); + + // Nothing authorized this proposer but its own identity. + BEAST_EXPECT(!env.le(keylet::signerList(target.id()))); + + auto const sle = env.le(keylet::txProposal(proposalID)); + if (!BEAST_EXPECT(sle)) + return; + BEAST_EXPECT(sle->getAccountID(sfOwner) == target.id()); + BEAST_EXPECT( + sle->getFieldObject(sfProposedTransaction).getAccountID(sfAccount) == target.id()); + + // The ticket and the proposal's increments are on the same account. + BEAST_EXPECT(ownerCount(env, target) == 1 + proposal::kProposalOwnerCount); + + env(proposal::cancel(target, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + // Released exactly once: the unused ticket is all that is left, and it + // is still the target's to spend. + BEAST_EXPECT(ownerCount(env, target) == 1); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + } + + // When the proposed transaction is initiated by a Delegate, that delegate + // is a target account too (On-Chain Cosigner spec §7.2): the proposal seeks its + // authorization, so it may refuse. + void + testDelegateCancel(FeatureBitset features) + { + testcase("proposed transaction's delegate cancels"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const carol{"carol"}; // the proposed transaction's Delegate + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, carol, bob); + env.close(); + + auto const delegatedPayload = [&](std::uint32_t ticketSeq) { + json::Value payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + payload[sfDelegate.getJsonName()] = carol.human(); + return payload; + }; + + // Naming carol in the payload is the whole of her claim: preclaim + // reads the Delegate as-is and never asks whether the delegation + // exists. Granting one here would make the cases below pass for a + // second reason and stop covering that, so pin its absence. + BEAST_EXPECT(!env.le(keylet::delegate(target.id(), carol.id()))); + + // The Delegate named in the payload may cancel. + { + std::uint32_t const ticketSeq = env.seq(target) + 1; + uint256 const proposalID = + makeProposal(env, alice, target, bob, delegatedPayload(ticketSeq)); + + env(proposal::cancel(carol, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // A Delegate widens, not replaces, the target: the proposed + // transaction's Account may still cancel. + { + std::uint32_t const ticketSeq = env.seq(target) + 1; + uint256 const proposalID = + makeProposal(env, alice, target, bob, delegatedPayload(ticketSeq)); + + env(proposal::cancel(target, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + } + + // While the proposal is live, nobody else may cancel it — not even the + // destination of the proposed payment (unlike a Check, whose destination + // may cancel it). + void + testThirdPartyNoPermission(FeatureBitset features) + { + testcase("third party cannot cancel live proposal"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; // destination of the proposed payment + env.fund(XRP(10000), alice, target, bob); + env.close(); + + uint256 const proposalID = makeProposal(env, alice, target, bob); + + env(proposal::cancel(bob, proposalID), Ter(tecNO_PERMISSION)); + env.close(); + + // The proposal survives and the Owner's reserve is still held. + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + + // Once the proposal's Expiration has passed it is terminal: closed to new + // signatures, so anyone may delete it and release the Owner's reserve. + void + testExpiredPermissionlessCancel(FeatureBitset features) + { + testcase("anyone cancels expired proposal"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + // A LastLedgerSequence far beyond this test keeps that arm of the + // terminal rule inert: only the Expiration arm may fire here. + json::Value payload = proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + payload[sfLastLedgerSequence.getJsonName()] = env.current()->seq() + 1000; + + env(proposal::create(alice, payload, proposal::expiration(env, 100s))); + env.close(); + uint256 const proposalID = keylet::txProposal(target.id(), ticketSeq).key; + + // Still live: a third party may not touch it. + env(proposal::cancel(bob, proposalID), Ter(tecNO_PERMISSION)); + env.close(); + + // Jump past the expiration. Whether the proposal is terminal is + // judged against the parent ledger's close time, the same rule + // Expiration follows everywhere. + env.close(env.now() + 200s); + + env(proposal::cancel(bob, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + // The target's unspent ticket survives its proposal. + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + } + + // A proposal whose proposed transaction has run out of ledgers — the + // ledger has moved past its LastLedgerSequence — is just as terminal as + // an expired one. The boundary mirrors the ordinary transactor's + // tefMAX_LEDGER rule: at view.seq() == LastLedgerSequence the payload can + // still be submitted, so the proposal is still live; only at view.seq() > + // LastLedgerSequence does it become terminal. + void + testLastLedgerSequenceTerminal(FeatureBitset features) + { + testcase("anyone cancels proposal past its LastLedgerSequence"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + std::uint32_t const lastLedgerSeq = env.current()->seq() + 3; + json::Value payload = proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + payload[sfLastLedgerSequence.getJsonName()] = lastLedgerSeq; + + env(proposal::create(alice, payload, proposal::expiration(env, 1000s))); + env.close(); + uint256 const proposalID = keylet::txProposal(target.id(), ticketSeq).key; + + // Close until the open ledger sits exactly at the bound. The payload + // could still be submitted in this ledger, so the proposal is not + // yet terminal and a third party still cannot cancel it. + while (env.current()->seq() < lastLedgerSeq) + env.close(); + BEAST_EXPECT(env.current()->seq() == lastLedgerSeq); + env(proposal::cancel(bob, proposalID), Ter(tecNO_PERMISSION)); + env.close(); + + // One ledger past the bound the payload can never be submitted, so + // the proposal is terminal and anyone may cancel it. + BEAST_EXPECT(env.current()->seq() == lastLedgerSeq + 1); + env(proposal::cancel(bob, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // The Expiration boundary: a proposal is terminal already in the first + // ledger whose parent close time equals its Expiration (the >= rule + // Expiration follows everywhere), not merely after it. + void + testExpirationBoundary(FeatureBitset features) + { + testcase("expiration boundary is inclusive"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + // Recorded close times land on multiples of the close time + // resolution, so pick the expiration on that grid and later aim a + // close at it exactly. Env::close(tp) sets the clock to + // tp + resolution - 1s and the accept path rounds to the nearest + // multiple, so a target of E - resolution + 1s closes at exactly E. + auto const resolution = env.closed()->header().closeTimeResolution; + NetClock::time_point const lastClose = env.current()->parentCloseTime(); + BEAST_EXPECT((lastClose.time_since_epoch() % resolution).count() == 0); + + NetClock::time_point const expiryTime = lastClose + 20 * resolution; + std::uint32_t const expiration = expiryTime.time_since_epoch().count(); + + env(proposal::create( + alice, + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq), + expiration)); + env.close(); + uint256 const proposalID = keylet::txProposal(target.id(), ticketSeq).key; + + // One grid step before the expiration the proposal is still live, + // even at the adjacent tick. + env.close(expiryTime - 2 * resolution + 1s); + BEAST_EXPECT(env.current()->parentCloseTime() == expiryTime - resolution); + env(proposal::cancel(bob, proposalID), Ter(tecNO_PERMISSION)); + + // The first ledger whose parent close time equals the Expiration is + // terminal. + env.close(expiryTime - resolution + 1s); + BEAST_EXPECT(env.current()->parentCloseTime() == expiryTime); + + env(proposal::cancel(bob, proposalID)); + env.close(); + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // A proposal turning terminal only widens who may cancel: the Owner and + // the target can always do it, before and after. + void + testOwnerCancelTerminal(FeatureBitset features) + { + testcase("owner cancels expired proposal"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + env(proposal::create( + alice, + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq), + proposal::expiration(env, 100s))); + env.close(); + uint256 const proposalID = keylet::txProposal(target.id(), ticketSeq).key; + + env.close(env.now() + 200s); + + env(proposal::cancel(alice, proposalID)); + env.close(); + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // A proposed Batch reserves more increments than an ordinary proposal; + // cancellation must release exactly that larger amount. + void + testBatchProposalCancel(FeatureBitset features) + { + testcase("cancel batch proposal releases its larger reserve"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + auto const seq = env.seq(target); + json::Value const payload = proposal::unsignedBatch( + env, + target, + ticketSeq, + tfAllOrNothing, + { + proposal::innerTx(pay(target, bob, XRP(1)), seq), + proposal::innerTx(pay(target, bob, XRP(1)), seq + 1), + }); + + env(proposal::create(alice, payload, proposal::expiration(env, 1000s))); + env.close(); + + uint256 const proposalID = keylet::txProposal(target.id(), ticketSeq).key; + BEAST_EXPECT(ownerCount(env, alice) == proposal::kBatchProposalOwnerCount); + + env(proposal::cancel(target, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // The Cancel transaction itself goes through the ordinary transaction + // machinery: it may spend a ticket of its submitter, and a target whose + // authority lives in a SignerList may cancel by multi-signing. + void + testCancelSubmissionForms(FeatureBitset features) + { + testcase("ticketed and multisigned cancels"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + Account const daria{"daria"}; + env.fund(XRP(10000), alice, target, bob, carol, daria); + env.close(); + + // The target's authority comes from a SignerList. Alice is on it so + // it may create the proposal this test then cancels. + env(signers(target, 2, {{alice, 1}, {carol, 1}, {daria, 1}})); + env.close(); + + // Cancel spending a ticket of the canceller. + { + uint256 const proposalID = makeProposal(env, alice, target, bob); + + std::uint32_t const cancelTicket = proposal::createTicket(env, alice); + + env(proposal::cancel(alice, proposalID), ticket::Use(cancelTicket)); + env.close(); + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + } + + // Cancel multi-signed for the target by its signer quorum. + { + uint256 const proposalID = makeProposal(env, alice, target, bob); + + env(proposal::cancel(target, proposalID), + Msig(carol, daria), + Fee(env.current()->fees().base * 3)); + env.close(); + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + } + + // Multi-signing for the target is not the same as acting for oneself. + // Carol is on the target's SignerList, which is enough to create a + // proposal against the target, yet §7.2 grants cancellation only to + // the Owner and to the target itself. So an account may propose but + // not un-propose — cancelling still has to come from the target, by + // the quorum route the block above takes. + { + uint256 const proposalID = makeProposal(env, alice, target, bob); + + env(proposal::cancel(carol, proposalID), Ter(tecNO_PERMISSION)); + env.close(); + + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + } + + // A Cancel works as an inner Batch transaction, where each inner runs its + // own preclaim at inner-apply time against the accumulated batch view. + void + testCancelInsideBatch(FeatureBitset features) + { + testcase("cancel as an inner batch transaction"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + // A multi-account batch: bob submits the outer Batch and the target + // authorizes its inner Cancel through a BatchSigners entry. + { + uint256 const proposalID = makeProposal(env, alice, target, bob); + + auto const seq = env.seq(bob); + auto const batchFee = batch::calcBatchFee(env, 1, 2); + env(batch::outer(bob, seq, batchFee, tfAllOrNothing), + batch::Inner(pay(bob, target, XRP(1)), seq + 1), + batch::Inner(proposal::cancel(target, proposalID), env.seq(target)), + batch::Sig(target)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // Two inner Cancels of the same proposal: the second finds nothing at + // its own preclaim. Under tfAllOrNothing that failure discards the + // whole batch, so the proposal survives with the reserve still held. + { + uint256 const proposalID = makeProposal(env, alice, target, bob); + + auto const seq = env.seq(target); + auto const batchFee = batch::calcBatchFee(env, 0, 2); + env(batch::outer(target, seq, batchFee, tfAllOrNothing), + batch::Inner(proposal::cancel(target, proposalID), seq + 1), + batch::Inner(proposal::cancel(target, proposalID), seq + 2)); + env.close(); + + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + + // Under tfIndependent the first inner's deletion stands and only + // the second fails. + auto const seq2 = env.seq(target); + env(batch::outer(target, seq2, batchFee, tfIndependent), + batch::Inner(proposal::cancel(target, proposalID), seq2 + 1), + batch::Inner(proposal::cancel(target, proposalID), seq2 + 2)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + + // Pin the two inner results from the closed ledger: the second + // Cancel failed at its own preclaim with tecNO_ENTRY. A tef from + // deeper in the apply pipeline would have kept it out of the + // ledger entirely. + json::Value params; + params[jss::ledger_index] = env.closed()->seq(); + params[jss::transactions] = true; + params[jss::expand] = true; + auto const jrr = env.rpc("json", "ledger", to_string(params)); + int success = 0; + int noEntry = 0; + for (auto const& txn : jrr[jss::result][jss::ledger][jss::transactions]) + { + if (txn[jss::TransactionType] == jss::TransactionProposalCancel) + { + auto const result = + txn[jss::metaData][sfTransactionResult.getJsonName()].asString(); + success += (result == "tesSUCCESS") ? 1 : 0; + noEntry += (result == "tecNO_ENTRY") ? 1 : 0; + } + } + BEAST_EXPECT(success == 1); + BEAST_EXPECT(noEntry == 1); + } + } + + // The Cancel transaction itself may have its fee sponsored, but it is not + // on the reserve-sponsorship allow-list (it creates nothing to reserve). + void + testSponsoredCancel(FeatureBitset features) + { + testcase("sponsored cancels"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const payer{"payer"}; + env.fund(XRP(10000), alice, target, bob, payer); + env.close(); + proposal::authorizeProposer(env, target, alice); + + uint256 const proposalID = makeProposal(env, alice, target, bob); + + // Reserve sponsorship is rejected for this transaction type. + env(proposal::cancel(alice, proposalID), + Fee(XRP(1)), + sponsor::As(payer, spfSponsorReserve), + Sig(sfSponsorSignature, payer), + Ter(temINVALID_FLAG)); + env.close(); + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + + // Fee sponsorship works: the sponsor pays, the canceller does not. + auto const before = env.balance(alice); + auto const payerBefore = env.balance(payer); + env(proposal::cancel(alice, proposalID), + Fee(XRP(1)), + sponsor::As(payer, spfSponsorFee), + Sig(sfSponsorSignature, payer)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + BEAST_EXPECT(env.balance(alice) == before); + BEAST_EXPECT(env.balance(payer) == payerBefore - XRP(1)); + } + + // A proposal's reserve may be sponsored, in which case deleteProposal has + // to release it against the account recorded in the entry's Sponsor rather + // than against the Owner. Creation and sponsorship transfer are covered by + // the Create suite; this is the release side of the same bookkeeping. + void + testSponsoredReserveCancel(FeatureBitset features) + { + testcase("cancel releases a sponsored reserve to its sponsor"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; // the proposer, and the proposal's Owner + Account const target{"target"}; + Account const bob{"bob"}; + Account const backer{"backer"}; // sponsors alice's proposal reserve + env.fund(XRP(10000), alice, target, bob, backer); + env.close(); + proposal::authorizeProposer(env, target, alice); + + // An ordinary proposal, cancelled by its Owner. + { + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + env(proposal::create( + alice, + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq), + proposal::expiration(env, 1000s)), + sponsor::As(backer, spfSponsorReserve), + Sig(sfSponsorSignature, backer), + proposal::verify::create()); + env.close(); + + uint256 const proposalID = keylet::txProposal(target.id(), ticketSeq).key; + BEAST_EXPECT(sponsoringOwnerCount(env, backer) == proposal::kProposalOwnerCount); + + env(proposal::cancel(alice, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + // The sponsorship unwinds from both ends: alice stops owning the + // increments and backer stops covering them. Neither is left + // holding a reserve for an object that no longer exists. + BEAST_EXPECT(ownerCount(env, alice) == 0); + BEAST_EXPECT(sponsoredOwnerCount(env, alice) == 0); + BEAST_EXPECT(sponsoringOwnerCount(env, backer) == 0); + BEAST_EXPECT(ownerCount(env, backer) == 0); + } + + // A proposed Batch holds the larger reserve, and here the target + // cancels rather than the Owner — so the release is shown to follow + // the Sponsor recorded on the entry, not anything about who submitted + // the Cancel. + { + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + auto const seq = env.seq(target); + json::Value const payload = proposal::unsignedBatch( + env, + target, + ticketSeq, + tfAllOrNothing, + { + proposal::innerTx(pay(target, bob, XRP(1)), seq), + proposal::innerTx(pay(target, bob, XRP(1)), seq + 1), + }); + + env(proposal::create(alice, payload, proposal::expiration(env, 1000s)), + sponsor::As(backer, spfSponsorReserve), + Sig(sfSponsorSignature, backer), + proposal::verify::create()); + env.close(); + + uint256 const proposalID = keylet::txProposal(target.id(), ticketSeq).key; + BEAST_EXPECT(sponsoringOwnerCount(env, backer) == proposal::kBatchProposalOwnerCount); + + env(proposal::cancel(target, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + BEAST_EXPECT(sponsoredOwnerCount(env, alice) == 0); + BEAST_EXPECT(sponsoringOwnerCount(env, backer) == 0); + } + } + + // deleteProposal removes the entry from its Owner's directory using the + // page hint stored in the entry's OwnerNode, so a proposal that is not on + // the first page has to be just as removable. + void + testCancelFromLaterDirectoryPage(FeatureBitset features) + { + testcase("cancel a proposal listed on a later directory page"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(100000), alice, target, bob); + env.close(); + + // Fill alice's first directory page exactly, so the proposal she + // creates next is listed on the second one. Tickets are the cheapest + // filler: one directory entry and one owner-count increment each, + // whereas a proposal costs one entry but five increments. + auto const pageEntries = static_cast(kDirNodeMaxEntries); + env(ticket::create(alice, pageEntries)); + env.close(); + BEAST_EXPECT(ownerCount(env, alice) == pageEntries); + + uint256 const proposalID = makeProposal(env, alice, target, bob); + + // Without this the setup could stop spanning pages — if the page size + // changed, say — and the test would quietly become testOwnerCancel. + auto const sle = env.le(keylet::txProposal(proposalID)); + if (!BEAST_EXPECT(sle && sle->getFieldU64(sfOwnerNode) != 0)) + return; + + env(proposal::cancel(alice, proposalID)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + // Only the proposal left; the tickets that filled the page are still + // alice's, so the release was scoped to the one entry. + BEAST_EXPECT(ownerCount(env, alice) == pageEntries); + } + + // What the ledger holds has to be visible to clients, and reported gone + // once cancelled: ledger_entry resolves the proposal by the (target, + // ticket) pair it is keyed on and by raw index, and account_objects lists + // it for the Owner — never for the target, whose directory it was never in. + void + testRpcVisibility(FeatureBitset features) + { + testcase("RPC visibility of a proposal"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + + std::uint32_t const ticketSeq = env.seq(target) + 1; + uint256 const proposalID = makeProposal(env, alice, target, bob); + + auto const ledgerEntry = [&env](json::Value const& proposalParams) { + json::Value params; + params[jss::ledger_index] = jss::validated; + params[jss::transaction_proposal] = proposalParams; + return env.rpc("json", "ledger_entry", to_string(params))[jss::result]; + }; + + json::Value byPair; + byPair[jss::account] = target.human(); + byPair[jss::ticket_seq] = ticketSeq; + + auto const accountObjects = [&env](Account const& account) { + json::Value params; + params[jss::account] = account.human(); + params[jss::type] = jss::transaction_proposal; + return env.rpc( + "json", "account_objects", to_string(params))[jss::result][jss::account_objects]; + }; + + // Found by the pair the key is derived from, and by that key directly. + { + auto const jrr = ledgerEntry(byPair); + BEAST_EXPECT(jrr[jss::index] == to_string(proposalID)); + BEAST_EXPECT( + jrr[jss::node][sfLedgerEntryType.getJsonName()] == jss::TransactionProposal); + BEAST_EXPECT(jrr[jss::node][sfOwner.getJsonName()] == alice.human()); + + json::Value params; + params[jss::ledger_index] = jss::validated; + params[jss::index] = to_string(proposalID); + auto const byIndex = env.rpc("json", "ledger_entry", to_string(params))[jss::result]; + BEAST_EXPECT(byIndex[jss::node] == jrr[jss::node]); + } + + // Each required field, in turn, left out — an absent field is a + // malformed request whichever one it is. Only a field that is present + // and unusable gets the more specific complaint. + { + json::Value noTicket; + noTicket[jss::account] = target.human(); + BEAST_EXPECT(ledgerEntry(noTicket)[jss::error] == "malformedRequest"); + + json::Value noAccount; + noAccount[jss::ticket_seq] = ticketSeq; + BEAST_EXPECT(ledgerEntry(noAccount)[jss::error] == "malformedRequest"); + + json::Value badAccount = byPair; + badAccount[jss::account] = "not an account"; + BEAST_EXPECT(ledgerEntry(badAccount)[jss::error] == "malformedAddress"); + } + + // Listed for the proposer, who owns it, and not for the target. + BEAST_EXPECT(accountObjects(alice).size() == 1); + BEAST_EXPECT( + accountObjects(alice)[0u][sfLedgerEntryType.getJsonName()] == jss::TransactionProposal); + BEAST_EXPECT(accountObjects(target).size() == 0); + + env(proposal::cancel(alice, proposalID)); + env.close(); + + // Gone. The handler reports the index it computed either way, so a + // client can tell "nothing at this key" from "malformed request". + { + auto const jrr = ledgerEntry(byPair); + BEAST_EXPECT(jrr[jss::error] == "entryNotFound"); + BEAST_EXPECT(!jrr.isMember(jss::node)); + BEAST_EXPECT(jrr[jss::index] == to_string(proposalID)); + } + BEAST_EXPECT(accountObjects(alice).size() == 0); + } + + // The Cancel transaction is not delegable: a Cancel carrying sfDelegate is + // rejected at preflight, and the permission cannot even be granted. + void + testDelegatedCancelRejected(FeatureBitset features) + { + testcase("cancel cannot be delegated"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const carol{"carol"}; + env.fund(XRP(10000), alice, target, carol); + env.close(); + proposal::authorizeProposer(env, target, alice); + + uint256 const proposalID = makeProposal(env, alice, target, alice); + + env(delegate::set(target, carol, {"TransactionProposalCancel"}), Ter(temMALFORMED)); + env.close(); + + env(proposal::cancel(target, proposalID), delegate::As(carol), Ter(temINVALID)); + env.close(); + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + } + + // A required auxiliary co-signer of the proposed transaction — here a + // LoanSet's explicit Counterparty — has its authorization sought by the + // proposal, yet §7.2 grants it no cancellation rights: it is neither the + // Owner nor the target. + void + testCounterpartyCannotCancel(FeatureBitset features) + { + testcase("proposed transaction's counterparty cannot cancel"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const borrower{"borrower"}; + Account const carol{"carol"}; // the LoanSet's Counterparty + env.fund(XRP(10000), alice, borrower, carol); + env.close(); + proposal::authorizeProposer(env, borrower, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, borrower); + + json::Value payload = + proposal::unsignedPayload(env, loan::set(borrower, uint256{1}, 1'000), ticketSeq); + payload[sfCounterparty.getJsonName()] = carol.human(); + + env(proposal::create(alice, payload, proposal::expiration(env, 1000s))); + env.close(); + uint256 const proposalID = keylet::txProposal(borrower.id(), ticketSeq).key; + + env(proposal::cancel(carol, proposalID), Ter(tecNO_PERMISSION)); + env.close(); + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + + // The target itself may, of course initiate the Cancel transaction. + env(proposal::cancel(borrower, proposalID)); + env.close(); + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + } + + // Being named in the payload is never itself a claim to cancel: preclaim's + // permitted set is a closed comparison against the Owner, the payload's + // Account and its Delegate, so a Sponsor is a third party like any other + // (§7.2). It is the most interested third party there is — it has agreed to + // pay the fee when the payload is submitted — which is what makes it worth + // stating. What the sponsorship does not change is either half of the rule: + // no permission while the proposal is live, and the same permission as + // everyone else once it is terminal. + void + testSponsorCannotCancel(FeatureBitset features) + { + testcase("proposed transaction's sponsor cannot cancel"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const backer{"backer"}; // pays the payload's fee + env.fund(XRP(10000), alice, target, bob, backer); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + json::Value tx = pay(target, bob, XRP(1)); + tx[sfSponsor.getJsonName()] = backer.human(); + tx[sfSponsorFlags.getJsonName()] = spfSponsorFee; + json::Value payload = proposal::unsignedPayload(env, tx, ticketSeq); + + // A LastLedgerSequence far beyond this test keeps that arm of the + // terminal rule inert: only the Expiration arm may fire below. + payload[sfLastLedgerSequence.getJsonName()] = env.current()->seq() + 1000; + + env(proposal::create(alice, payload, proposal::expiration(env, 100s))); + env.close(); + uint256 const proposalID = keylet::txProposal(target.id(), ticketSeq).key; + + // Live: the sponsor is refused, and refusing costs the proposal nothing. + env(proposal::cancel(backer, proposalID), Ter(tecNO_PERMISSION)); + env.close(); + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + + // Terminal: the permission question stops being asked, so the account + // refused a moment ago may now delete it — and the reserve it releases + // is still the Owner's, not the sponsor's. + env.close(env.now() + 200s); + + env(proposal::cancel(backer, proposalID)); + env.close(); + BEAST_EXPECT(!env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // Cancel may not spend the ticket the proposal is keyed to. The + // reservation is only for the proposed transaction (On-Chain Cosigner spec §4.2.1); + // a target that wants to revoke both the proposal and the ticket must + // cancel first, then spend the ticket separately. + void + testCancelCannotSpendReservedTicket(FeatureBitset features) + { + testcase("cancel cannot consume the reserved ticket"); + + using namespace jtx; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + // Standalone: the Cancel is held as a retry and neither object is + // touched. + { + std::uint32_t const ticketSeq = env.seq(target) + 1; + uint256 const proposalID = makeProposal(env, alice, target, bob); + + env(proposal::cancel(target, proposalID), + ticket::Use(ticketSeq), + Ter(terTICKET_RESERVED)); + env.close(); + + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + BEAST_EXPECT(ownerCount(env, target) == 2); + } + + // The same attempt as an inner Batch transaction: tfAllOrNothing + // discards the whole batch, so the sibling payment does not land. + { + std::uint32_t const ticketSeq = env.seq(target) + 1; + uint256 const proposalID = makeProposal(env, alice, target, bob); + + auto const seq = env.seq(target); + auto const bobBefore = env.balance(bob); + auto const batchFee = batch::calcBatchFee(env, 0, 2); + env(batch::outer(target, seq, batchFee, tfAllOrNothing), + batch::Inner(proposal::cancel(target, proposalID), 0, ticketSeq), + batch::Inner(pay(target, bob, XRP(1)), seq + 1)); + env.close(); + + BEAST_EXPECT(env.le(keylet::txProposal(proposalID))); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(env.balance(bob) == bobBefore); + BEAST_EXPECT(ownerCount(env, alice) == 2 * proposal::kProposalOwnerCount); + BEAST_EXPECT(ownerCount(env, target) == 3); + } + } + + // A proposal must not nest proposal machinery: proposing a + // TransactionProposalCancel is rejected outright. A signer-quorum target + // that wants to refuse a proposal multi-signs a Cancel directly instead. + void + testCannotProposeCancel(FeatureBitset features) + { + testcase("a cancel cannot itself be proposed"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + env.fund(XRP(10000), alice, target); + env.close(); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + std::uint32_t const expiration = proposal::expiration(env, 1000s); + + // Directly proposed. + { + json::Value const payload = + proposal::unsignedPayload(env, proposal::cancel(target, uint256{1}), ticketSeq); + + env(proposal::create(alice, payload, expiration), Ter(temINVALID)); + env.close(); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + } + + // Wrapped inside a proposed Batch: nesting must not be evadable by + // one level of indirection (On-Chain Cosigner spec §5.3.1). Each + // wrapped transaction is well-formed for its type, so the rejection + // can only come from the nesting rule itself. The sibling inner uses + // a sequence distinct from the wrapped transaction's, so the Batch is + // not rejected as redundant either. + auto const rejectWrapped = [&](json::Value const& wrapped, TER expected) { + json::Value const payload = proposal::unsignedBatch( + env, + target, + ticketSeq, + tfAllOrNothing, + {proposal::innerTx(pay(target, alice, XRP(1)), env.seq(target) + 1), wrapped}); + + env(proposal::create(alice, payload, expiration), Ter(expected)); + env.close(); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + }; + + // A well-formed inner TransactionProposalCancel. + rejectWrapped( + proposal::innerTx(proposal::cancel(target, uint256{1}), env.seq(target)), temINVALID); + + // A well-formed inner TransactionProposalCreate. + rejectWrapped( + proposal::innerTx( + proposal::create( + target, + proposal::unsignedPayload(env, pay(target, alice, XRP(1)), 1), + expiration), + env.seq(target)), + temINVALID); + + // An inner Batch is rejected at STTx construction (a Batch cannot + // contain a Batch), so Create surfaces temMALFORMED rather than the + // nesting rule's temINVALID. + { + json::Value nested; + nested[jss::TransactionType] = jss::Batch; + nested[jss::Account] = target.human(); + nested[jss::Flags] = tfAllOrNothing; + nested[jss::RawTransactions][0u][jss::RawTransaction] = + proposal::innerTx(pay(target, alice, XRP(1)), env.seq(target)); + nested[jss::RawTransactions][1u][jss::RawTransaction] = + proposal::innerTx(proposal::cancel(target, uint256{1}), env.seq(target)); + + rejectWrapped(proposal::innerTx(std::move(nested), env.seq(target)), temMALFORMED); + } + } + + void + run() override + { + using namespace jtx; + + FeatureBitset const all{testableAmendments()}; + + // Preflight + testDisabled(all); + testMalformed(all); + testDelegatedCancelRejected(all); + testCannotProposeCancel(all); + + // Preclaim + testNoEntry(all); + testThirdPartyNoPermission(all); + testExpiredPermissionlessCancel(all); + testExpirationBoundary(all); + testLastLedgerSequenceTerminal(all); + testCounterpartyCannotCancel(all); + testSponsorCannotCancel(all); + testCancelCannotSpendReservedTicket(all); + + // Apply + testOwnerCancel(all); + testTargetCancel(all); + testSelfProposalCancel(all); + testDelegateCancel(all); + testOwnerCancelTerminal(all); + testBatchProposalCancel(all); + testCancelSubmissionForms(all); + testCancelInsideBatch(all); + testSponsoredCancel(all); + testSponsoredReserveCancel(all); + testCancelFromLaterDirectoryPage(all); + + // RPC + testRpcVisibility(all); + } +}; + +BEAST_DEFINE_TESTSUITE(TransactionProposalCancel, app, xrpl); + +} // namespace xrpl::test diff --git a/src/test/app/TransactionProposalCreate_test.cpp b/src/test/app/TransactionProposalCreate_test.cpp index e736205756..145e825005 100644 --- a/src/test/app/TransactionProposalCreate_test.cpp +++ b/src/test/app/TransactionProposalCreate_test.cpp @@ -19,11 +19,13 @@ #include #include +#include #include #include #include #include #include +#include #include #include #include @@ -36,6 +38,7 @@ #include #include #include +#include #include #include // IWYU pragma: keep @@ -44,6 +47,7 @@ #include #include #include +#include #include namespace xrpl::test { @@ -163,7 +167,7 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite // Proposals do not nest. This payload also isn't a valid instance of // TransactionProposalCreate's own template (it lacks Expiration and // ProposedTransaction), so it fails STTx construction before ever - // reaching our own isProposalTx check. + // reaching our own nesting check. { json::Value tx = payload(); tx[jss::TransactionType] = "TransactionProposalCreate"; @@ -189,6 +193,49 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite reject(tx, temINVALID); } + // A Batch inner lacking TransactionType cannot be constructed as an + // STTx, so Create rejects the payload as temMALFORMED before the + // nesting walk runs at all. That inner would otherwise make the walk + // report temINVALID, so this pins the ordering. + { + json::Value missingType; + missingType[jss::Account] = target.human(); + missingType[jss::Flags] = tfInnerBatchTxn; + missingType[jss::Sequence] = env.seq(target); + missingType[jss::Fee] = "10"; + missingType[jss::SigningPubKey] = ""; + + reject( + proposal::unsignedBatch( + env, + target, + targetTicketSeq, + tfAllOrNothing, + { + proposal::innerTx(pay(target, bob, XRP(1)), env.seq(target) + 1), + missingType, + }), + temMALFORMED); + } + + // The walk rejects such an inner in its own right. Nothing submitted + // through Create can reach that guard — the case above shows STTx + // construction throwing first — so it is checked directly here, which + // also pins it for a caller that does not build an STTx beforehand. + { + STObject inner(sfRawTransaction); + inner.setAccountID(sfAccount, target.id()); + + STArray rawTxns(sfRawTransactions); + rawTxns.push_back(std::move(inner)); + + STObject batch(sfGeneric); + batch.setFieldU16(sfTransactionType, ttBATCH); + batch.setFieldArray(sfRawTransactions, rawTxns); + + BEAST_EXPECT(!xrpl::proposal::isValidProposalTxnType(batch)); + } + // The proposed transaction must be ticket-based: a missing // TicketSequence, or a live Sequence alongside it, is rejected. { diff --git a/src/test/app/TransactionProposalReservedTicket_test.cpp b/src/test/app/TransactionProposalReservedTicket_test.cpp new file mode 100644 index 0000000000..5aa659e27e --- /dev/null +++ b/src/test/app/TransactionProposalReservedTicket_test.cpp @@ -0,0 +1,1775 @@ +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include + +namespace xrpl::test { + +// While a TransactionProposal exists, the Ticket it is keyed to is reserved +// (On-Chain Cosigner spec §4.2.1): a transaction may consume it only if its payload matches +// the stored ProposedTransaction, so unrelated target-account activity cannot +// invalidate the proposal while signatures are being collected. When the +// matching transaction does consume the ticket, the ledger deletes the +// now-stale proposal and releases the reserve it holds against its Owner +// (On-Chain Cosigner spec §4.5). +struct TransactionProposalReservedTicket_test : public beast::unit_test::Suite +{ + // Create a live proposal owned by proposer holding the given payload. + static void + makeProposal(jtx::Env& env, jtx::Account const& proposer, json::Value const& payload) + { + using namespace jtx; + using namespace std::chrono_literals; + + Account const target = env.lookup(payload[jss::Account].asString()); + if (proposer.id() != target.id() && !env.le(keylet::signerList(target.id()))) + proposal::authorizeProposer(env, target, proposer); + + env(proposal::create(proposer, payload, proposal::expiration(env, 1000s))); + env.close(); + } + + // Rebuild a transaction from a copy of it whose fields have been edited. + // Editing an STTx in place would leave getTransactionID — and the + // signature verdict the HashRouter caches under it — describing the + // transaction before the edit. Going through the bytes rather than + // STTx{STObject&&} is what makes this work on a copy of an already + // templated transaction: applyTemplate refuses a defaulted field that is + // explicitly present, which such a copy carries, and serializing omits it. + static std::shared_ptr + rebuilt(STObject const& tx) + { + Serializer s; + tx.add(s); + return std::make_shared(SerialIter{s.slice()}); + } + + // A transaction that is not the proposed transaction cannot consume the + // reserved ticket: it fails with terTICKET_RESERVED and neither the + // ticket nor the proposal is touched. The target's sequence, and its + // other tickets, stay freely spendable. + void + testUnrelatedSpendBlocked(FeatureBitset features) + { + testcase("unrelated spend of a reserved ticket is blocked"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; // the proposer + Account const target{"target"}; // the account the proposal is for + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target, 2); + json::Value const payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + makeProposal(env, alice, payload); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + BEAST_EXPECT(ownerCount(env, target) == 3); + + // A different transaction type. + env(noop(target), ticket::Use(ticketSeq), Ter(terTICKET_RESERVED)); + // The proposed transaction with one payload field off: the amount, + // the destination, or the fee. Signature fields are the only + // latitude a submitter has (On-Chain Cosigner spec §4.2.1). + env(pay(target, bob, XRP(2)), ticket::Use(ticketSeq), Ter(terTICKET_RESERVED)); + env(pay(target, alice, XRP(1)), ticket::Use(ticketSeq), Ter(terTICKET_RESERVED)); + env(pay(target, bob, XRP(1)), + ticket::Use(ticketSeq), + Fee(env.current()->fees().base * 2), + Ter(terTICKET_RESERVED)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + BEAST_EXPECT(ownerCount(env, target) == 3); + + // The reservation binds one ticket, not the account: its sequence + // and its other tickets spend freely, and the proposal survives. + env(noop(target)); + env(noop(target), ticket::Use(ticketSeq + 1)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(ownerCount(env, target) == 2); + } + + // The proposal's own transaction consumes the reserved ticket through + // the ordinary submission path (On-Chain Cosigner spec §6.5). How it is signed is + // irrelevant to the match — signature fields are excluded — and applying + // it auto-deletes the proposal and refunds the Owner (On-Chain Cosigner spec §4.5). + void + testMatchingPayloadExecutes(FeatureBitset features) + { + testcase("matching payload consumes the ticket and cleans up"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + env.fund(XRP(10000), alice, target, bob, carol); + env.close(); + + // Single-signed with the target's own key: the stored payload has an + // empty SigningPubKey, the submission a populated one plus a + // TxnSignature. Both are signature fields, so they still match. + { + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + makeProposal( + env, alice, proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq)); + + auto const bobBefore = env.balance(bob); + env(pay(target, bob, XRP(1)), ticket::Use(ticketSeq)); + env.close(); + + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(1)); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + BEAST_EXPECT(ownerCount(env, target) == 1); + } + + // Multi-signed under the target's SignerList: the Signers array the + // submission carries is a signature field too. The payload's Fee was + // fixed at creation, so a proposer expecting multi-signed submission + // must budget for the signatures then (On-Chain Cosigner spec §4.2.1). + { + env(signers(target, 2, {{alice, 1}, {bob, 1}, {carol, 1}})); + env.close(); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + auto const msigFee = env.current()->fees().base * 3; + json::Value proposedTx = pay(target, bob, XRP(1)); + proposedTx[jss::Fee] = std::to_string(msigFee.drops()); + makeProposal(env, alice, proposal::unsignedPayload(env, proposedTx, ticketSeq)); + + auto const bobBefore = env.balance(bob); + env(pay(target, bob, XRP(1)), ticket::Use(ticketSeq), Msig(bob, carol), Fee(msigFee)); + env.close(); + + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(1)); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == 0); + // The consumed ticket's count is released; the SignerList set up + // above remains the target's only owned object. + BEAST_EXPECT(ownerCount(env, target) == 1); + } + } + + // Cancelling the proposal removes the reservation. terTICKET_RESERVED is + // a retry: the very transaction the reservation blocked is held and + // applies on its own once a Cancel frees the ticket. + void + testCancelFreesTicket(FeatureBitset features) + { + testcase("cancelling the proposal frees the ticket"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + + // After a Cancel, the ticket spends freely. + { + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + makeProposal( + env, alice, proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq)); + + env(proposal::cancel(alice, keylet::txProposal(target.id(), ticketSeq).key)); + env.close(); + + env(noop(target), ticket::Use(ticketSeq)); + env.close(); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + } + + // A blocked transaction is held for retry, and consumes the ticket + // by itself once the proposal is gone. Two closes: the held + // transaction and the Cancel land in the same ledger in salted + // canonical order, so the retry may run before the Cancel once more. + { + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + makeProposal( + env, alice, proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq)); + + env(noop(target), ticket::Use(ticketSeq), Ter(terTICKET_RESERVED)); + + env(proposal::cancel(alice, keylet::txProposal(target.id(), ticketSeq).key)); + env.close(); + env.close(); + + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + } + } + + // A ticket is consumed even when its transaction fails with a tec. The + // reservation lets only the matching payload get that far, and once the + // ticket is durably gone the proposal is cleaned up all the same. + void + testMatchingTecStillCleansUp(FeatureBitset features) + { + testcase("matching payload failing with tec still cleans up"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + // A proposed payment the target cannot fund. Creation only runs the + // payload's stateless checks, so the proposal stores it happily. + makeProposal( + env, + alice, + proposal::unsignedPayload(env, pay(target, bob, XRP(1'000'000)), ticketSeq)); + + env(pay(target, bob, XRP(1'000'000)), ticket::Use(ticketSeq), Ter(tecUNFUNDED_PAYMENT)); + env.close(); + + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + BEAST_EXPECT(ownerCount(env, target) == 1); + } + + // The reservation is keyed by target account and ticket, so another + // account spending its own ticket of the same numeric sequence is + // neither blocked nor does it touch the proposal. + void + testOtherAccountsSameNumberedTicket(FeatureBitset features) + { + testcase("other account's same-numbered ticket is unaffected"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + + // target and bob were funded together, so creating one ticket each + // in the same ledger gives their tickets the same numeric sequence. + std::uint32_t const targetTicketSeq = env.seq(target) + 1; + std::uint32_t const bobTicketSeq = env.seq(bob) + 1; + env(ticket::create(target, 1)); + env(ticket::create(bob, 1)); + env.close(); + BEAST_EXPECT(targetTicketSeq == bobTicketSeq); + + makeProposal( + env, alice, proposal::unsignedPayload(env, pay(target, bob, XRP(1)), targetTicketSeq)); + + env(noop(bob), ticket::Use(bobTicketSeq)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, targetTicketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(targetTicketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + + // The proposer may be the target itself. The reservation still binds it + // — its own unrelated spend is blocked — and the matching execution + // lands the ticket bookkeeping and the reserve release on the same + // account-root SLE within one ticketDelete call, pinning that aliasing. + void + testProposerIsTarget(FeatureBitset features) + { + testcase("proposer is the target account"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, bob); + env.close(); + + std::uint32_t const ticketSeq = proposal::createTicket(env, alice); + makeProposal( + env, alice, proposal::unsignedPayload(env, pay(alice, bob, XRP(1)), ticketSeq)); + + auto const sle = proposal::entry(env, alice, ticketSeq); + BEAST_EXPECT(sle && sle->getAccountID(sfOwner) == alice.id()); + // One Ticket plus the proposal's increments, all against alice. + BEAST_EXPECT(ownerCount(env, alice) == 1 + proposal::kProposalOwnerCount); + + env(noop(alice), ticket::Use(ticketSeq), Ter(terTICKET_RESERVED)); + env.close(); + BEAST_EXPECT(proposal::entry(env, alice, ticketSeq)); + + env(pay(alice, bob, XRP(1)), ticket::Use(ticketSeq)); + env.close(); + + BEAST_EXPECT(!proposal::entry(env, alice, ticketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // A proposed Batch: the reservation covers its outer ticket, only the + // identical Batch consumes it, and its deletion must release the larger + // reserve a Batch proposal holds (On-Chain Cosigner spec §4.4). + void + testBatchProposal(FeatureBitset features) + { + testcase("proposed Batch: only the identical Batch spends the ticket"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + // Build the payload once; the same JSON is stored in the proposal + // and later submitted, differing only by the signature env() adds. + auto const innerSeq = env.seq(target); + json::Value const payload = proposal::unsignedBatch( + env, + target, + ticketSeq, + tfAllOrNothing, + { + proposal::innerTx(pay(target, bob, XRP(1)), innerSeq), + proposal::innerTx(pay(target, bob, XRP(1)), innerSeq + 1), + }); + makeProposal(env, alice, payload); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kBatchProposalOwnerCount); + + // A different Batch spending the reserved outer ticket is blocked. + env(batch::outer(target, 0, batch::calcBatchFee(env, 0, 2), tfAllOrNothing), + batch::Inner(pay(target, bob, XRP(5)), innerSeq), + batch::Inner(pay(target, bob, XRP(5)), innerSeq + 1), + ticket::Use(ticketSeq), + Ter(terTICKET_RESERVED)); + env.close(); + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + + // The proposed Batch itself executes: outer ticket consumed, both + // inner payments applied, proposal deleted, larger reserve released. + auto const bobBefore = env.balance(bob); + env(payload); + env.close(); + + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(2)); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // An inner Batch transaction validates its ticket through the same path + // as a standalone one, so a reserved ticket blocks it too — an inner + // transaction carries tfInnerBatchTxn and a zero fee, so it can never + // match a stored payload, which may not carry that flag (On-Chain Cosigner spec + // §4.2.1). Under tfAllOrNothing the whole Batch is discarded. + void + testInnerBatchSpendBlocked(FeatureBitset features) + { + testcase("inner batch transaction cannot spend a reserved ticket"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + json::Value const payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + makeProposal(env, alice, payload); + + // The second inner transaction spends the reserved ticket — even + // carrying the very payment the proposal holds, its flag and fee + // make it a different payload, so it fails and tfAllOrNothing + // discards its sibling too. + auto const bobBefore = env.balance(bob); + auto const seq = env.seq(target); + env(batch::outer(target, seq, batch::calcBatchFee(env, 0, 2), tfAllOrNothing), + batch::Inner(pay(target, bob, XRP(1)), seq + 1), + batch::Inner(pay(target, bob, XRP(1)), 0, ticketSeq)); + env.close(); + + BEAST_EXPECT(env.balance(bob) == bobBefore); + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + + // The standalone matching payment still goes through. + env(pay(target, bob, XRP(1)), ticket::Use(ticketSeq)); + env.close(); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // A proposed multi-account Batch is stored with no BatchSigners: the + // participants' signatures are collected afterwards, and the submission + // that completes it carries them. BatchSigners is one of the containers + // payloadMatches excludes, which is exactly what lets the completed Batch + // still match what was stored. + void + testMultiAccountBatchCompletes(FeatureBitset features) + { + testcase("proposed multi-account Batch completes with BatchSigners"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + env.fund(XRP(10000), alice, target, bob, carol); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + // One inner from each of two accounts, so the Batch is unsubmittable + // without bob's BatchSigners entry. The second inner moves funds bob + // holds to a third account, so each inner is separately observable. + json::Value const payload = proposal::unsignedBatch( + env, + target, + ticketSeq, + tfAllOrNothing, + { + proposal::innerTx(pay(target, bob, XRP(1)), env.seq(target)), + proposal::innerTx(pay(bob, carol, XRP(2)), env.seq(bob)), + }); + makeProposal(env, alice, payload); + + auto const sle = proposal::entry(env, target, ticketSeq); + BEAST_EXPECT( + sle && !sle->getFieldObject(sfProposedTransaction).isFieldPresent(sfBatchSigners)); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kBatchProposalOwnerCount); + + // Submitted exactly as stored, the Batch cannot even preflight: a + // multi-account Batch demands an entry per participant. So excluding + // BatchSigners from the match is load-bearing rather than incidental — + // without it this proposal could never be completed at all. + env(payload, Ter(temBAD_SIGNER)); + env.close(); + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + + auto const bobBefore = env.balance(bob); + auto const carolBefore = env.balance(carol); + env(payload, batch::Sig(bob)); + env.close(); + + // Both inners applied: bob took XRP(1) from the target and paid + // XRP(2) out, and inner transactions carry no fee of their own. + BEAST_EXPECT(env.balance(carol) == carolBefore + XRP(2)); + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(1) - XRP(2)); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // A proposed LoanSet stores no CounterpartySignature — that is collected + // afterwards too — and the completed submission carrying one still + // matches. The payload's Fee is fixed at creation, so it has to budget for + // that signature up front: LoanSet charges an extra base fee per + // counterparty signature, and nothing can raise the fee later. + void + testCounterpartySignatureMatches(FeatureBitset features) + { + testcase("proposed LoanSet completes with a CounterpartySignature"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; // the borrower + Account const lender{"lender"}; // the LoanSet's Counterparty + env.fund(XRP(10000), alice, target, lender); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + json::Value tx = loan::set(target, uint256{1}, 1'000); + tx[sfCounterparty.getJsonName()] = lender.human(); + tx[jss::Fee] = std::to_string((env.current()->fees().base * 2).drops()); + json::Value const payload = proposal::unsignedPayload(env, tx, ticketSeq); + makeProposal(env, alice, payload); + + // The named LoanBroker does not exist, so this fails at preclaim — + // which is the point: getting as far as preclaim proves the added + // CounterpartySignature did not break the payload match, and a tec + // consumes the ticket and cleans the proposal up all the same. + env(payload, Sig(sfCounterpartySignature, lender), Ter(tecNO_ENTRY)); + env.close(); + + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // The last of the co-signature containers: a payload may name a fee + // sponsor at creation and collect that sponsor's signature later. Sponsor + // and SponsorFlags are ordinary payload fields, fixed from the start; + // only the SponsorSignature itself arrives at submission. + void + testSponsorSignatureMatches(FeatureBitset features) + { + testcase("proposed payload completes with a SponsorSignature"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const backer{"backer"}; // pays the payload's fee + env.fund(XRP(10000), alice, target, bob, backer); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + json::Value tx = pay(target, bob, XRP(1)); + tx[sfSponsor.getJsonName()] = backer.human(); + tx[sfSponsorFlags.getJsonName()] = spfSponsorFee; + json::Value const payload = proposal::unsignedPayload(env, tx, ticketSeq); + makeProposal(env, alice, payload); + + auto const sle = proposal::entry(env, target, ticketSeq); + BEAST_EXPECT( + sle && !sle->getFieldObject(sfProposedTransaction).isFieldPresent(sfSponsorSignature)); + + auto const fee = env.current()->fees().base; + auto const bobBefore = env.balance(bob); + auto const targetBefore = env.balance(target); + auto const backerBefore = env.balance(backer); + + env(payload, Sig(sfSponsorSignature, backer)); + env.close(); + + // The sponsor covered the fee, so the target parted with the payment + // amount and nothing beyond it. + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(1)); + BEAST_EXPECT(env.balance(target) == targetBefore - XRP(1)); + BEAST_EXPECT(env.balance(backer) == backerBefore - fee); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // A terminal proposal — expired, or past its payload's LastLedgerSequence + // — still reserves its ticket. canConsumeTicket never consults + // isTerminal: going terminal widens who may cancel a proposal, but it + // releases nothing, because the ticket is reserved for as long as the + // proposal exists (On-Chain Cosigner spec §4.2.1). Freeing the ticket + // takes a Cancel, which anyone may now send. + void + testTerminalStillReservesTicket(FeatureBitset features) + { + testcase("a terminal proposal still reserves its ticket"); + + using namespace jtx; + using namespace std::chrono_literals; + + // An unrelated spend stays blocked after the proposal expires. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + // A far-future LastLedgerSequence keeps that arm of the terminal + // rule inert, so the expiry alone makes the proposal terminal. + json::Value payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + payload[sfLastLedgerSequence.getJsonName()] = env.current()->seq() + 1000; + + env(proposal::create(alice, payload, proposal::expiration(env, 100s))); + env.close(); + + env.close(env.now() + 200s); + + env(noop(target), ticket::Use(ticketSeq), Ter(terTICKET_RESERVED)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + + // The same indifference to isTerminal in the other direction: the + // proposal's own payload still consumes the ticket after the proposal + // expires. An expired proposal is closed to new signatures, not to + // the transaction it already holds. A separate environment keeps the + // held retry from the block above out of this one. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + json::Value const payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + + env(proposal::create(alice, payload, proposal::expiration(env, 100s))); + env.close(); + + env.close(env.now() + 200s); + + auto const bobBefore = env.balance(bob); + env(payload); + env.close(); + + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(1)); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + } + + // A reserve-sponsored proposal cleaned up by its own payload consuming the + // ticket releases the reserve against the sponsor. deleteProposal follows + // the Sponsor recorded on the entry, so this path and the Cancel path land + // in the same place. + void + testSponsoredReserveAutoDelete(FeatureBitset features) + { + testcase("auto-deleting a sponsored proposal releases its sponsor"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const backer{"backer"}; // sponsors alice's proposal reserve + env.fund(XRP(10000), alice, target, bob, backer); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + json::Value const payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + + env(proposal::create(alice, payload, proposal::expiration(env, 1000s)), + sponsor::As(backer, spfSponsorReserve), + Sig(sfSponsorSignature, backer), + proposal::verify::create()); + env.close(); + + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + BEAST_EXPECT(sponsoredOwnerCount(env, alice) == proposal::kProposalOwnerCount); + BEAST_EXPECT(sponsoringOwnerCount(env, backer) == proposal::kProposalOwnerCount); + + env(payload); + env.close(); + + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == 0); + BEAST_EXPECT(sponsoredOwnerCount(env, alice) == 0); + BEAST_EXPECT(sponsoringOwnerCount(env, backer) == 0); + BEAST_EXPECT(ownerCount(env, backer) == 0); + } + + // Deleting the target account sweeps its Tickets without consuming them + // as a sequence proxy, so the reservation does not block it. A proposal + // keyed to a swept ticket can never execute and is cleaned up with it, + // refunding the proposer (On-Chain Cosigner spec §4.5). + void + testTargetAccountDeleted(FeatureBitset features) + { + testcase("deleting the target account deletes the proposal"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + makeProposal( + env, alice, proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + + incLgrSeqForAccDel(env, target); + env(acctdelete(target, bob), Fee(env.current()->fees().increment)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::account(target.id()))); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // The sweep above, with more than one proposal and more than one Owner. + // Each reserve has to return to the account that put it up, which means the + // sweep cannot stop at the first proposal it reaches and cannot settle the + // refunds against a single account. Two Owners also make the directory + // separation concrete: the sweep walks the target's owner directory while + // each deletion mutates a different one. + void + testTargetDeletionRefundsEachOwner(FeatureBitset features) + { + testcase("target deletion refunds each proposal's own owner"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; // Owner of the proposal on the first ticket + Account const bob{"bob"}; // Owner of the one on the second + Account const target{"target"}; + Account const dest{"dest"}; + env.fund(XRP(10000), alice, bob, target, dest); + env.close(); + + // Both proposers on one SignerList. makeProposal's own authorization + // step adds a list for the first proposer only and then finds one + // present, so the second proposer would be unauthorized without this. + env(signers(target, 1, {{alice, 1}, {bob, 1}})); + env.close(); + + std::uint32_t const firstTicket = proposal::createTicket(env, target, 2); + std::uint32_t const secondTicket = firstTicket + 1; + + makeProposal( + env, alice, proposal::unsignedPayload(env, pay(target, dest, XRP(1)), firstTicket)); + makeProposal( + env, bob, proposal::unsignedPayload(env, pay(target, dest, XRP(2)), secondTicket)); + + BEAST_EXPECT(proposal::entry(env, target, firstTicket)); + BEAST_EXPECT(proposal::entry(env, target, secondTicket)); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + BEAST_EXPECT(ownerCount(env, bob) == proposal::kProposalOwnerCount); + + incLgrSeqForAccDel(env, target); + env(acctdelete(target, dest), Fee(env.current()->fees().increment)); + env.close(); + + BEAST_EXPECT(!env.le(keylet::account(target.id()))); + BEAST_EXPECT(!proposal::entry(env, target, firstTicket)); + BEAST_EXPECT(!proposal::entry(env, target, secondTicket)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(firstTicket)))); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(secondTicket)))); + + // Each Owner got its own reserve back, and only its own. + BEAST_EXPECT(ownerCount(env, alice) == 0); + BEAST_EXPECT(ownerCount(env, bob) == 0); + } + + // A TransactionProposal blocks its Owner's account deletion (On-Chain Cosigner spec + // §4.5). Beyond the spec requirement, this blocker is what guarantees + // the AccountDelete ticket sweep never deletes a proposal out of the + // very owner directory it is iterating: any proposal reached through a + // swept ticket is necessarily owned by an account other than the one + // being deleted. + void + testProposalBlocksOwnerAccountDelete(FeatureBitset features) + { + testcase("proposal blocks its owner's account deletion"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; // proposer, tries to delete itself + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + makeProposal( + env, alice, proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq)); + + incLgrSeqForAccDel(env, alice); + env(acctdelete(alice, bob), Fee(env.current()->fees().increment), Ter(tecHAS_OBLIGATIONS)); + env.close(); + + BEAST_EXPECT(env.le(keylet::account(alice.id()))); + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + + // An account holding a blocker needs to be able to find out why its + // deletion failed, so account_objects has to agree with the transactor + // about what counts as one. + json::Value params; + params[jss::account] = alice.human(); + params[jss::deletion_blockers_only] = true; + auto const jrr = env.rpc("json", "account_objects", to_string(params))[jss::result]; + BEAST_EXPECT(jrr[jss::account_objects].size() == 1); + BEAST_EXPECT( + jrr[jss::account_objects][0u][sfLedgerEntryType.getJsonName()] == + jss::TransactionProposal); + } + + // A proposal reserves a ticket, not the target's signing authority: the + // target stays free to rewrite its SignerList while signatures are being + // collected. Signatures gathered against the old list then no longer + // authorize the payload, and the payload's Fee was fixed at creation, so + // it cannot buy more of them. None of these failures claims a fee or + // applies anything, so the ticket stays reserved and the payload keeps + // whatever signing routes the rewritten list still leaves it + // (On-Chain Cosigner spec §4.2.1). + void + testSignerListChangedUnderProposal(FeatureBitset features) + { + testcase("target's SignerList changes under a stored proposal"); + + using namespace jtx; + using namespace std::chrono_literals; + + // A signer dropped from the list can no longer contribute. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + env.fund(XRP(10000), alice, target, bob, carol); + env.close(); + + env(signers(target, 2, {{alice, 1}, {bob, 1}, {carol, 1}})); + env.close(); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + auto const msigFee = env.current()->fees().base * 3; + json::Value proposedTx = pay(target, bob, XRP(1)); + proposedTx[jss::Fee] = std::to_string(msigFee.drops()); + json::Value const payload = proposal::unsignedPayload(env, proposedTx, ticketSeq); + makeProposal(env, alice, payload); + + // The target drops carol after the proposal is stored. + env(signers(target, 2, {{alice, 1}, {bob, 1}})); + env.close(); + + env(payload, Msig(bob, carol), Ter(tefBAD_SIGNATURE)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + + // Only the dropped signer is gone, not the route: the signers + // still on the list meet the unchanged quorum, and because two + // signatures is what the Fee budgeted for, the payload completes. + env(payload, Msig(alice, bob)); + env.close(); + + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // A quorum raised beyond what the fixed Fee can pay for. That closes + // the multisign route the Fee was budgeted for, but not the proposal: + // a SignerList does not revoke the master key, and signing singly + // costs less than the Fee already covers. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + env.fund(XRP(10000), alice, target, bob, carol); + env.close(); + + env(signers(target, 1, {{alice, 1}, {bob, 1}, {carol, 1}})); + env.close(); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + auto const msigFee = env.current()->fees().base * 2; + json::Value proposedTx = pay(target, bob, XRP(1)); + proposedTx[jss::Fee] = std::to_string(msigFee.drops()); + json::Value const payload = proposal::unsignedPayload(env, proposedTx, ticketSeq); + makeProposal(env, alice, payload); + + env(signers(target, 3, {{alice, 1}, {bob, 1}, {carol, 1}})); + env.close(); + + // The single signature the Fee paid for no longer reaches quorum. + env(payload, Msig(bob), Ter(tefBAD_QUORUM)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + + // And the three signatures that would reach it cost more than the + // Fee fixed at creation. They do satisfy the raised quorum, and + // checkSign runs ahead of checkFee, so nothing is masked here: a + // three-signer multisign is priced at four base fees against the + // two the payload fixed, and checkFee is what rejects it. + env(payload, Msig(alice, bob, carol), Ter(telINSUF_FEE_P)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + + // What closed is the multisign route, not the proposal. Raising a + // quorum does not revoke the master key, and one signature is + // priced at a single base fee, well inside what the Fee covers. + auto const bobBefore = env.balance(bob); + env(payload, Sig(target)); + env.close(); + + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(1)); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + } + + // The signature-presence checks that TapProposal skips at creation time + // are not skipped at submission (On-Chain Cosigner spec §5.3.1.2). Each is + // a tem, so the submission claims no fee: the ticket keeps its + // reservation and the proposal stays put. + void + testDeferredSignatureChecksFireOnSubmission(FeatureBitset features) + { + testcase("deferred signature-presence checks fire at submission"); + + using namespace jtx; + using namespace std::chrono_literals; + + // LoanSet's CounterpartySignature. testCounterpartySignatureMatches + // covers the completed submission; this covers its absence, which is + // what LoanSet::preflight stops checking under TapProposal. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; // the borrower + Account const lender{"lender"}; + env.fund(XRP(10000), alice, target, lender); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + json::Value tx = loan::set(target, uint256{1}, 1'000); + tx[sfCounterparty.getJsonName()] = lender.human(); + json::Value const payload = proposal::unsignedPayload(env, tx, ticketSeq); + makeProposal(env, alice, payload); + + env(payload, Ter(temBAD_SIGNER)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + + // An account-level SponsorshipTransfer's SponsorSignature. Only the + // account-level form defers this: it needs no ObjectID, which is what + // distinguishes it from object-level sponsorship. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const backer{"backer"}; + env.fund(XRP(10000), alice, target, backer); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + json::Value tx = sponsor::transfer(target, tfSponsorshipCreate); + tx[sfSponsor.getJsonName()] = backer.human(); + tx[sfSponsorFlags.getJsonName()] = spfSponsorReserve; + json::Value const payload = proposal::unsignedPayload(env, tx, ticketSeq); + makeProposal(env, alice, payload); + + env(payload, Ter(temMALFORMED)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + + // Reassignment is the other way into that check: creating and + // reassigning are gated by one account-level condition, so this pins + // the flags the check answers for rather than a second code path. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const backer{"backer"}; + env.fund(XRP(10000), alice, target, backer); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + json::Value tx = sponsor::transfer(target, tfSponsorshipReassign); + tx[sfSponsor.getJsonName()] = backer.human(); + tx[sfSponsorFlags.getJsonName()] = spfSponsorReserve; + json::Value const payload = proposal::unsignedPayload(env, tx, ticketSeq); + makeProposal(env, alice, payload); + + env(payload, Ter(temMALFORMED)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + } + + // What a tem submission costs the proposal. Creation cannot store a + // payload that fails its own preflight, so the only payload rejected + // unaltered is one whose check creation defers: here a multi-account + // Batch, submitted without the BatchSigners entry it will eventually be + // given. payloadMatches is asserted first, so the rejection cannot be read + // as the test having submitted something else. + void + testTemSubmissionConsumesNothing(FeatureBitset features) + { + testcase("a tem submission consumes neither ticket nor proposal"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + env.fund(XRP(10000), alice, target, bob, carol); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + json::Value const payload = proposal::unsignedBatch( + env, + target, + ticketSeq, + tfAllOrNothing, + { + proposal::innerTx(pay(target, bob, XRP(1)), env.seq(target)), + proposal::innerTx(pay(bob, carol, XRP(2)), env.seq(bob)), + }); + makeProposal(env, alice, payload); + + auto const sle = proposal::entry(env, target, ticketSeq); + if (!BEAST_EXPECT(sle)) + return; + + // The transaction about to be rejected is the proposal's own payload. + auto const jt = env.jt(payload); + if (!BEAST_EXPECT(jt.stx)) + return; + BEAST_EXPECT( + xrpl::proposal::payloadMatches(sle->getFieldObject(sfProposedTransaction), *jt.stx)); + + auto const targetBefore = env.balance(target); + auto const bobBefore = env.balance(bob); + auto const carolBefore = env.balance(carol); + + env(payload, Ter(temBAD_SIGNER)); + env.close(); + + // A tem is rejected before it can claim a fee, so the target paid + // nothing for the attempt and no inner transaction ran. + BEAST_EXPECT(env.balance(target) == targetBefore); + BEAST_EXPECT(env.balance(bob) == bobBefore); + BEAST_EXPECT(env.balance(carol) == carolBefore); + + // The proposal is still stored, still holding its reserve, and the + // ticket still exists. + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kBatchProposalOwnerCount); + + // And the proposal still completes on that same ticket once the + // signatures it was waiting for arrive, which is what makes the + // rejection above a no-op rather than a loss. + env(payload, batch::Sig(bob)); + env.close(); + + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(1) - XRP(2)); + BEAST_EXPECT(env.balance(carol) == carolBefore + XRP(2)); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // Skipping Batch::preflightSigValidated at creation defers every shape of + // BatchSigners error, not just an absent array. + // testMultiAccountBatchCompletes covers the absent case; these are the + // malformed ones, each rejected at submission with the ticket and the + // proposal untouched. + void + testMalformedBatchSignersOnSubmission(FeatureBitset features) + { + testcase("malformed BatchSigners rejected at submission"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + env.fund(XRP(10000), alice, target, bob, carol); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + // Inners from three accounts, so the completed Batch needs an entry + // from each of bob and carol. + json::Value const payload = proposal::unsignedBatch( + env, + target, + ticketSeq, + tfAllOrNothing, + { + proposal::innerTx(pay(target, bob, XRP(1)), env.seq(target)), + proposal::innerTx(pay(bob, carol, XRP(2)), env.seq(bob)), + proposal::innerTx(pay(carol, bob, XRP(1)), env.seq(carol)), + }); + makeProposal(env, alice, payload); + + auto const stillTicketReservedForProposal = [&] { + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kBatchProposalOwnerCount); + }; + + // Batch signers are checked against the required signers positionally, + // both sorted by account ID, so which malformation a given array trips + // depends on this ordering. Pin it: if these fixed jtx accounts ever + // sort differently, that should fail here rather than quietly move the + // cases below onto a different branch. The outer account is included + // because the first case below relies on it sorting ahead of bob. + BEAST_EXPECT(target.id() < alice.id() && alice.id() < carol.id() && carol.id() < bob.id()); + + // The outer account may not sign for itself. + env(payload, batch::Sig(target, bob), Ter(temBAD_SIGNER)); + env.close(); + stillTicketReservedForProposal(); + + // A duplicated signer. It has to be carol, the first required signer: + // a duplicated bob would be rejected as a mismatch against carol + // before the array's second entry is ever examined. + env(payload, batch::Sig(carol, carol), Ter(temBAD_SIGNER)); + env.close(); + stillTicketReservedForProposal(); + + // An unsorted pair. batch::Sig sorts what it is given, so the array + // has to be swapped after the fact to express this. Swapping the two + // required signers would not reach the sort check either, so the pair + // is carol and alice: carol matches, then alice sorts below her. + { + auto jt = env.jt(payload, batch::Sig(alice, carol)); + auto const first = jt.jv[sfBatchSigners.jsonName][0u]; + auto const second = jt.jv[sfBatchSigners.jsonName][1u]; + jt.jv[sfBatchSigners.jsonName][0u] = second; + jt.jv[sfBatchSigners.jsonName][1u] = first; + + env(jt.jv, Ter(temBAD_SIGNER)); + env.close(); + stillTicketReservedForProposal(); + } + + // Correctly formed, the same proposal still completes: the rejections + // above are about the signers array, not the stored payload. + env(payload, batch::Sig(bob, carol)); + env.close(); + + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + // Create rejects a payload whose LastLedgerSequence is at or before the + // ledger Create itself applies in (tecEXPIRED). A bound that passes after + // the proposal exists is a different matter: at view.seq() == + // LastLedgerSequence the payload still submits + // (the transactor's tefMAX_LEDGER rule is view.seq() > bound), and the + // proposal's own terminal rule uses the same boundary, so the proposal + // is live in that same ledger. Only once view.seq() has advanced past + // the bound does the payload become unsubmittable — as a tef it leaves + // the ticket and the proposal in place, and Cancel is the only exit. + void + testPayloadLastLedgerSequencePasses(FeatureBitset features) + { + testcase("payload's LastLedgerSequence passes while the proposal waits"); + + using namespace jtx; + using namespace std::chrono_literals; + + // One ledger past the bound: the payload can no longer be submitted, + // and being a tef it takes neither the ticket nor the proposal with + // it. Cancel is the only exit left. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + std::uint32_t const lastLedgerSeq = env.current()->seq() + 5; + json::Value payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + payload[sfLastLedgerSequence.getJsonName()] = lastLedgerSeq; + makeProposal(env, alice, payload); + + while (env.current()->seq() <= lastLedgerSeq) + env.close(); + + env(payload, Ter(tefMAX_LEDGER)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + + // Exactly at the bound: still live and still submittable — the + // proposal's terminal rule and the transactor's tefMAX_LEDGER rule + // both trigger only strictly past the bound, so the payload applies + // in this last valid ledger and cleans up as usual. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + std::uint32_t const lastLedgerSeq = env.current()->seq() + 5; + json::Value payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + payload[sfLastLedgerSequence.getJsonName()] = lastLedgerSeq; + makeProposal(env, alice, payload); + + while (env.current()->seq() < lastLedgerSeq) + env.close(); + + auto const bobBefore = env.balance(bob); + env(payload); + env.close(); + + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(1)); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + } + + // Matching the stored payload is not authority to submit it. SigningPubKey + // and TxnSignature are excluded from the match, so the submission's key is + // checked on its own terms — and the target may change which keys count + // after the proposal is stored. + void + testSubmissionSigningAuthority(FeatureBitset features) + { + testcase("submission signed by an unauthorized or disabled key"); + + using namespace jtx; + using namespace std::chrono_literals; + + // A key with no authority over the target. The payload still matches; + // it is the authority that is missing. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + json::Value const payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + makeProposal(env, alice, payload); + + env(payload, Sig(bob), Ter(tefBAD_AUTH)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + + // The target disables its master key after the proposal is stored. The + // key that would have submitted the payload no longer can. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + json::Value const payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + makeProposal(env, alice, payload); + + // Disabling the master key requires another way in first. + env(regkey(target, bob)); + env(fset(target, asfDisableMaster), Sig(target)); + env.close(); + // Signed with the master key the proposal was made under, the + // payload is now refused. The key has to be named explicitly: + // left to autofill, the submission would take the regular key + // set above and succeed, proving nothing. + env(payload, Sig(target), Ter(tefMASTER_DISABLED)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + + // The regular key set above is a route the payload can still take, + // so this proposal is stalled rather than dead. + auto const bobBefore = env.balance(bob); + env(payload, Sig(bob)); + env.close(); + + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(1)); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + } + + // The payload's Fee is chosen at creation and is part of what a submission + // must match, so it can never be raised. A proposal that under-budgets the + // signatures it will need is therefore permanently unsubmittable. Both + // unsignedBatch's numSigners parameter and the LoanSet Fee multiplier + // exist to avoid this; these are the cases that walk into it. + void + testFeeFixedAtCreationCannotCoverSignatures(FeatureBitset features) + { + testcase("fee fixed at creation cannot cover the signatures needed"); + + using namespace jtx; + using namespace std::chrono_literals; + + // A multi-account Batch that budgeted for no BatchSigners at all. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + Account const carol{"carol"}; + env.fund(XRP(10000), alice, target, bob, carol); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + json::Value const payload = proposal::unsignedBatch( + env, + target, + ticketSeq, + tfAllOrNothing, + { + proposal::innerTx(pay(target, bob, XRP(1)), env.seq(target)), + proposal::innerTx(pay(bob, carol, XRP(2)), env.seq(bob)), + }, + 0u); + makeProposal(env, alice, payload); + + // Bare, it cannot preflight: the signer entry is still required. + env(payload, Ter(temBAD_SIGNER)); + env.close(); + + // With the entry it preflights, and then cannot pay for it. + env(payload, batch::Sig(bob), Ter(telINSUF_FEE_P)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kBatchProposalOwnerCount); + } + + // A proposed LoanSet left at the single base fee, which cannot cover + // the CounterpartySignature it is going to need. unsignedPayload + // stamps the single base fee when the caller does not choose one, + // while LoanSet prices each counterparty signer at one further base + // fee, so the payload is short by exactly one. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; // the borrower + Account const lender{"lender"}; + env.fund(XRP(10000), alice, target, lender); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + json::Value tx = loan::set(target, uint256{1}, 1'000); + tx[sfCounterparty.getJsonName()] = lender.human(); + json::Value const payload = proposal::unsignedPayload(env, tx, ticketSeq); + makeProposal(env, alice, payload); + + // The shortfall is what the case turns on, so pin it — read off + // the stored proposal rather than the JSON built above. + auto const sle = proposal::entry(env, target, ticketSeq); + if (!BEAST_EXPECT(sle)) + return; + BEAST_EXPECT( + sle->getFieldObject(sfProposedTransaction).getFieldAmount(sfFee).xrp() == + env.current()->fees().base); + + env(payload, Sig(sfCounterpartySignature, lender), Ter(telINSUF_FEE_P)); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + } + + // SigningPubKey and TxnSignature are excluded from the payload match, so a + // submission may carry whatever it likes in them and still match. Nothing + // about holding a proposal excuses the content of those fields: a key that + // is not a key is refused in preflight, and a well-formed signature that + // does not verify in preflight2. + // + // These go through xrpl::apply because jtx would overwrite the signature + // fields when it signs, and the submit path rejects a bad signature itself + // before a transactor sees it. The OpenView is discarded either way, so + // only the returned ter and applied flag are evidence here; the + // reservation surviving a tem is covered by + // testTemSubmissionConsumesNothing. + void + testSubmissionSignatureContent(FeatureBitset features) + { + testcase("submission carrying malformed signature content"); + + using namespace jtx; + using namespace std::chrono_literals; + + // A SigningPubKey that is not a public key at all. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + json::Value const payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + makeProposal(env, alice, payload); + + auto const jt = env.jt(payload); + if (!BEAST_EXPECT(jt.stx)) + return; + + STObject mutated{*jt.stx}; + std::string const badKey{"badkey"}; + mutated.at(sfSigningPubKey) = makeSlice(badKey); + auto const stx = rebuilt(mutated); + + env.app().getOpenLedger().modify([&](OpenView& view, beast::Journal j) { + auto const result = xrpl::apply(env.app(), view, *stx, TapNone, j); + BEAST_EXPECT(result.ter == temBAD_SIGNATURE); + BEAST_EXPECT(!result.applied); + return result.applied; + }); + } + + // A signature that is structurally fine but was made over a different + // key's identity: bob's signature presented under the target's + // SigningPubKey. This is the case a malformed-key check cannot catch. + { + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + env.fund(XRP(10000), alice, target, bob); + env.close(); + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + json::Value const payload = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + makeProposal(env, alice, payload); + + auto const jt = env.jt(payload); + auto const jtOther = env.jt(payload, Sig(bob)); + if (!BEAST_EXPECT(jt.stx && jtOther.stx)) + return; + + STObject mutated{*jt.stx}; + Blob const otherSig = jtOther.stx->getFieldVL(sfTxnSignature); + mutated.at(sfTxnSignature) = makeSlice(otherSig); + auto const stx = rebuilt(mutated); + + env.app().getOpenLedger().modify([&](OpenView& view, beast::Journal j) { + auto const result = xrpl::apply(env.app(), view, *stx, TapNone, j); + BEAST_EXPECT(result.ter == temINVALID); + BEAST_EXPECT(!result.applied); + return result.applied; + }); + } + } + + // The other way a fixed Fee stops being enough: the Fee does not change, + // the ledger's price does. This one is not fatal, and it is not + // telINSUF_FEE_P either — the queue holds the payload at terQUEUED rather + // than rejecting it, and applies it once the escalated fee drops. So an + // expensive ledger delays a completed proposal; it does not strand one. + void + testOpenLedgerFeeEscalation(FeatureBitset features) + { + testcase("open-ledger fee escalation only delays the payload"); + + using namespace jtx; + using namespace std::chrono_literals; + + auto cfg = envconfig([](std::unique_ptr cfg) { + auto& section = cfg->section(Sections::kTransactionQueue); + // Price above two transactions per ledger, not the standalone + // default of 1000, so the three fillers below escalate the fee. + section.set(Keys::kMinimumTxnInLedgerStandalone, "2"); + return cfg; + }); + Env env{*this, std::move(cfg), features}; + + Account const alice{"alice"}; + Account const target{"target"}; + Account const bob{"bob"}; + + // Funded one at a time: with a target of two transactions per ledger, + // funding them together would itself escalate the fee. + env.fund(XRP(10000), alice); + env.close(); + env.fund(XRP(10000), target); + env.close(); + env.fund(XRP(10000), bob); + env.close(); + + proposal::authorizeProposer(env, target, alice); + + std::uint32_t const ticketSeq = proposal::createTicket(env, target); + + // Name the Fee rather than letting fillFee supply it: the whole case + // turns on the payload being priced at the base fee while the ledger + // around it is not. + auto const baseFee = env.current()->fees().base; + json::Value proposedTx = pay(target, bob, XRP(1)); + proposedTx[jss::Fee] = std::to_string(baseFee.drops()); + json::Value const payload = proposal::unsignedPayload(env, proposedTx, ticketSeq); + makeProposal(env, alice, payload); + + // Fill the open ledger past its target so the next transaction is + // priced above the base fee the payload was created with. + env(noop(alice)); + env(noop(alice)); + env(noop(alice)); + + // Both sides of the shortfall, so terQUEUED below rests on asserted + // facts rather than on the tuning above having worked: the stored + // payload is priced at the base fee, and the open ledger now charges + // more than that. + auto const sleProposal = proposal::entry(env, target, ticketSeq); + if (!BEAST_EXPECT(sleProposal)) + return; + BEAST_EXPECT( + sleProposal->getFieldObject(sfProposedTransaction).getFieldAmount(sfFee).xrp() == + baseFee); + BEAST_EXPECT( + toDrops(env.app().getTxQ().getMetrics(*env.current()).openLedgerFeeLevel, baseFee) > + baseFee); + + auto const bobBefore = env.balance(bob); + env(payload, Ter(terQUEUED)); + + // Still intact while queued: nothing has been applied yet. + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + + // Closing the ledger returns the price to the base fee, and that is + // what lets the queue apply a payload whose Fee was fixed there: it + // goes in unchanged, resolving the proposal as any match would. + env.close(); + + BEAST_EXPECT( + toDrops(env.app().getTxQ().getMetrics(*env.current()).openLedgerFeeLevel, baseFee) == + baseFee); + + BEAST_EXPECT(env.balance(bob) == bobBefore + XRP(1)); + BEAST_EXPECT(!proposal::entry(env, target, ticketSeq)); + BEAST_EXPECT(!env.le(keylet::ticket(target.id(), SeqProxy::rawTicket(ticketSeq)))); + BEAST_EXPECT(ownerCount(env, alice) == 0); + } + + void + run() override + { + using namespace jtx; + auto const all = testableAmendments(); + testUnrelatedSpendBlocked(all); + testMatchingPayloadExecutes(all); + testCancelFreesTicket(all); + testMatchingTecStillCleansUp(all); + testOtherAccountsSameNumberedTicket(all); + testProposerIsTarget(all); + testBatchProposal(all); + testMultiAccountBatchCompletes(all); + testCounterpartySignatureMatches(all); + testSponsorSignatureMatches(all); + testInnerBatchSpendBlocked(all); + testTerminalStillReservesTicket(all); + testSponsoredReserveAutoDelete(all); + testTargetAccountDeleted(all); + testTargetDeletionRefundsEachOwner(all); + testProposalBlocksOwnerAccountDelete(all); + testSignerListChangedUnderProposal(all); + testDeferredSignatureChecksFireOnSubmission(all); + testTemSubmissionConsumesNothing(all); + testMalformedBatchSignersOnSubmission(all); + testPayloadLastLedgerSequencePasses(all); + testSubmissionSigningAuthority(all); + testFeeFixedAtCreationCannotCoverSignatures(all); + testSubmissionSignatureContent(all); + testOpenLedgerFeeEscalation(all); + } +}; + +BEAST_DEFINE_TESTSUITE(TransactionProposalReservedTicket, app, xrpl); + +} // namespace xrpl::test diff --git a/src/test/jtx/impl/proposal.cpp b/src/test/jtx/impl/proposal.cpp index 79d82d8f28..997a21025c 100644 --- a/src/test/jtx/impl/proposal.cpp +++ b/src/test/jtx/impl/proposal.cpp @@ -47,6 +47,16 @@ create(Account const& proposer, json::Value const& proposedTx, std::uint32_t exp return jv; } +json::Value +cancel(Account const& account, uint256 const& proposalID) +{ + json::Value jv; + jv[jss::TransactionType] = jss::TransactionProposalCancel; + jv[jss::Account] = account.human(); + jv[sfProposalID.jsonName] = to_string(proposalID); + return jv; +} + json::Value unsignedPayload(Env const& env, json::Value tx, std::uint32_t ticketSeq) { diff --git a/src/test/jtx/proposal.h b/src/test/jtx/proposal.h index 2703fd6046..b158a3d19a 100644 --- a/src/test/jtx/proposal.h +++ b/src/test/jtx/proposal.h @@ -4,6 +4,7 @@ #include #include +#include #include #include #include @@ -40,6 +41,16 @@ using xrpl::proposal::kProposalOwnerCount; json::Value create(Account const& proposer, json::Value const& proposedTx, std::uint32_t expiration); +/** + * @brief Build a TransactionProposalCancel naming the proposal to delete. + * + * @param account The account requesting the cancellation. + * @param proposalID The ID of the TransactionProposal to cancel. + * @return The TransactionProposalCancel JSON object. + */ +json::Value +cancel(Account const& account, uint256 const& proposalID); + /** * @brief Conditions that check what a proposal transaction did to the ledger. * diff --git a/src/tests/libxrpl/protocol_autogen/transactions/TransactionProposalCancelTests.cpp b/src/tests/libxrpl/protocol_autogen/transactions/TransactionProposalCancelTests.cpp new file mode 100644 index 0000000000..5d10867617 --- /dev/null +++ b/src/tests/libxrpl/protocol_autogen/transactions/TransactionProposalCancelTests.cpp @@ -0,0 +1,146 @@ +// Auto-generated unit tests for transaction TransactionProposalCancel + + +#include + +#include + +#include +#include +#include +#include +#include + +#include + +namespace xrpl::transactions { + +// 1 & 4) Set fields via builder setters, build, then read them back via +// wrapper getters. After build(), validate() should succeed. +TEST(TransactionsTransactionProposalCancelTests, BuilderSettersRoundTrip) +{ + // Generate a deterministic keypair for signing + auto const [publicKey, secretKey] = + generateKeyPair(KeyType::Secp256k1, generateSeed("testTransactionProposalCancel")); + + // Common transaction fields + auto const accountValue = calcAccountID(publicKey); + std::uint32_t const sequenceValue = 1; + auto const feeValue = canonical_AMOUNT(); + + // Transaction-specific field values + auto const proposalIDValue = canonical_UINT256(); + + TransactionProposalCancelBuilder builder{ + accountValue, + proposalIDValue, + sequenceValue, + feeValue + }; + + // Set optional fields + + auto tx = builder.build(publicKey, secretKey); + + std::string reason; + EXPECT_TRUE(tx.validate(reason)) << reason; + + // Verify signing was applied + EXPECT_FALSE(tx.getSigningPubKey().empty()); + EXPECT_TRUE(tx.hasTxnSignature()); + + // Verify common fields + EXPECT_EQ(tx.getAccount(), accountValue); + EXPECT_EQ(tx.getSequence(), sequenceValue); + EXPECT_EQ(tx.getFee(), feeValue); + + // Verify required fields + { + auto const& expected = proposalIDValue; + auto const actual = tx.getProposalID(); + expectEqualField(expected, actual, "sfProposalID"); + } + + // Verify optional fields +} + +// 2 & 4) Start from an STTx, construct a builder from it, build a new wrapper, +// and verify all fields match. +TEST(TransactionsTransactionProposalCancelTests, BuilderFromStTxRoundTrip) +{ + // Generate a deterministic keypair for signing + auto const [publicKey, secretKey] = + generateKeyPair(KeyType::Secp256k1, generateSeed("testTransactionProposalCancelFromTx")); + + // Common transaction fields + auto const accountValue = calcAccountID(publicKey); + std::uint32_t const sequenceValue = 2; + auto const feeValue = canonical_AMOUNT(); + + // Transaction-specific field values + auto const proposalIDValue = canonical_UINT256(); + + // Build an initial transaction + TransactionProposalCancelBuilder initialBuilder{ + accountValue, + proposalIDValue, + sequenceValue, + feeValue + }; + + + auto initialTx = initialBuilder.build(publicKey, secretKey); + + // Create builder from existing STTx + TransactionProposalCancelBuilder builderFromTx{initialTx.getSTTx()}; + + auto rebuiltTx = builderFromTx.build(publicKey, secretKey); + + std::string reason; + EXPECT_TRUE(rebuiltTx.validate(reason)) << reason; + + // Verify common fields + EXPECT_EQ(rebuiltTx.getAccount(), accountValue); + EXPECT_EQ(rebuiltTx.getSequence(), sequenceValue); + EXPECT_EQ(rebuiltTx.getFee(), feeValue); + + // Verify required fields + { + auto const& expected = proposalIDValue; + auto const actual = rebuiltTx.getProposalID(); + expectEqualField(expected, actual, "sfProposalID"); + } + + // Verify optional fields +} + +// 3) Verify wrapper throws when constructed from wrong transaction type. +TEST(TransactionsTransactionProposalCancelTests, WrapperThrowsOnWrongTxType) +{ + // Build a valid transaction of a different type + auto const [pk, sk] = + generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongType")); + auto const account = calcAccountID(pk); + + AccountSetBuilder wrongBuilder{account, 1, canonical_AMOUNT()}; + auto wrongTx = wrongBuilder.build(pk, sk); + + EXPECT_THROW(TransactionProposalCancel{wrongTx.getSTTx()}, std::runtime_error); +} + +// 4) Verify builder throws when constructed from wrong transaction type. +TEST(TransactionsTransactionProposalCancelTests, BuilderThrowsOnWrongTxType) +{ + // Build a valid transaction of a different type + auto const [pk, sk] = + generateKeyPair(KeyType::Secp256k1, generateSeed("testWrongTypeBuilder")); + auto const account = calcAccountID(pk); + + AccountSetBuilder wrongBuilder{account, 1, canonical_AMOUNT()}; + auto wrongTx = wrongBuilder.build(pk, sk); + + EXPECT_THROW(TransactionProposalCancelBuilder{wrongTx.getSTTx()}, std::runtime_error); +} + + +} diff --git a/src/tests/libxrpl/tx/ProposalHelpers.cpp b/src/tests/libxrpl/tx/ProposalHelpers.cpp index 1b148535fe..51ecdca336 100644 --- a/src/tests/libxrpl/tx/ProposalHelpers.cpp +++ b/src/tests/libxrpl/tx/ProposalHelpers.cpp @@ -15,7 +15,7 @@ namespace xrpl::test { namespace { -// A bare STObject carrying only sfTransactionType. isValidProposal accepts an +// A bare STObject carrying only sfTransactionType. isValidProposalTxnType accepts an // STObject (not an STTx) precisely so that this file's defense-in-depth // checks can be tested without the surrounding STTx format validation. inline STObject @@ -45,7 +45,7 @@ batchWrapping(STObject inner) // The happy path — an ordinary Payment is independently submittable. TEST(ProposalHelpers, plain_payment_is_valid) { - EXPECT_TRUE(proposal::isValidProposal(txOfType(ttPAYMENT))); + EXPECT_TRUE(proposal::isValidProposalTxnType(txOfType(ttPAYMENT))); } // A nested TransactionProposalCreate. In practice STTx construction rejects @@ -54,16 +54,16 @@ TEST(ProposalHelpers, plain_payment_is_valid) // drift apart. TEST(ProposalHelpers, nested_proposal_is_rejected) { - EXPECT_FALSE(proposal::isValidProposal(txOfType(ttTRANSACTION_PROPOSAL_CREATE))); + EXPECT_FALSE(proposal::isValidProposalTxnType(txOfType(ttTRANSACTION_PROPOSAL_CREATE))); } // Any pseudo-transaction — see STTx::isPseudoTx. Also normally caught earlier // by STTx construction / preflight0. TEST(ProposalHelpers, pseudo_tx_is_rejected) { - EXPECT_FALSE(proposal::isValidProposal(txOfType(ttAMENDMENT))); - EXPECT_FALSE(proposal::isValidProposal(txOfType(ttFEE))); - EXPECT_FALSE(proposal::isValidProposal(txOfType(ttUNL_MODIFY))); + EXPECT_FALSE(proposal::isValidProposalTxnType(txOfType(ttAMENDMENT))); + EXPECT_FALSE(proposal::isValidProposalTxnType(txOfType(ttFEE))); + EXPECT_FALSE(proposal::isValidProposalTxnType(txOfType(ttUNL_MODIFY))); } // tfInnerBatchTxn marks a transaction as an inner leg of an enclosing Batch, @@ -74,7 +74,7 @@ TEST(ProposalHelpers, inner_batch_flag_is_rejected) { STObject tx = txOfType(ttPAYMENT); tx.setFieldU32(sfFlags, tfInnerBatchTxn); - EXPECT_FALSE(proposal::isValidProposal(tx)); + EXPECT_FALSE(proposal::isValidProposalTxnType(tx)); } // A Flags value that is present but does not include tfInnerBatchTxn must @@ -83,26 +83,27 @@ TEST(ProposalHelpers, other_flags_are_accepted) { STObject tx = txOfType(ttPAYMENT); tx.setFieldU32(sfFlags, tfFullyCanonicalSig); - EXPECT_TRUE(proposal::isValidProposal(tx)); + EXPECT_TRUE(proposal::isValidProposalTxnType(tx)); } // A Batch wrapping a plain inner is fine — the loop is only there to catch // specifically forbidden inner types. TEST(ProposalHelpers, batch_with_plain_inner_is_valid) { - EXPECT_TRUE(proposal::isValidProposal(batchWrapping(txOfType(ttPAYMENT)))); + EXPECT_TRUE(proposal::isValidProposalTxnType(batchWrapping(txOfType(ttPAYMENT)))); } // A Batch whose inner is itself a proposal must be rejected. TEST(ProposalHelpers, batch_with_nested_proposal_inner_is_rejected) { - EXPECT_FALSE(proposal::isValidProposal(batchWrapping(txOfType(ttTRANSACTION_PROPOSAL_CREATE)))); + EXPECT_FALSE( + proposal::isValidProposalTxnType(batchWrapping(txOfType(ttTRANSACTION_PROPOSAL_CREATE)))); } // A Batch whose inner is a pseudo-transaction must be rejected. TEST(ProposalHelpers, batch_with_pseudo_inner_is_rejected) { - EXPECT_FALSE(proposal::isValidProposal(batchWrapping(txOfType(ttAMENDMENT)))); + EXPECT_FALSE(proposal::isValidProposalTxnType(batchWrapping(txOfType(ttAMENDMENT)))); } // A Batch with no sfRawTransactions field skips the inner-loop entirely. @@ -110,7 +111,7 @@ TEST(ProposalHelpers, batch_with_pseudo_inner_is_rejected) // helper (the field is optional at the STObject level) and should hold. TEST(ProposalHelpers, batch_without_raw_transactions_is_valid) { - EXPECT_TRUE(proposal::isValidProposal(txOfType(ttBATCH))); + EXPECT_TRUE(proposal::isValidProposalTxnType(txOfType(ttBATCH))); } } // namespace xrpl::test