diff --git a/src/test/app/TransactionProposalCreate_test.cpp b/src/test/app/TransactionProposalCreate_test.cpp index ca9be5b9b9..f6909e2e77 100644 --- a/src/test/app/TransactionProposalCreate_test.cpp +++ b/src/test/app/TransactionProposalCreate_test.cpp @@ -3,118 +3,85 @@ #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 // IWYU pragma: keep +#include #include +#include #include +#include namespace xrpl::test { struct TransactionProposalCreate_test : public beast::unit_test::Suite { - // A TransactionProposalCreate carrying an unsigned proposed transaction. - static json::Value - proposalCreate( - jtx::Account const& proposer, - json::Value const& proposedTx, - std::uint32_t expiration) - { - json::Value jv; - jv[jss::TransactionType] = "TransactionProposalCreate"; - jv[jss::Account] = proposer.human(); - jv[sfProposedTransaction.getJsonName()] = proposedTx; - jv[sfExpiration.getJsonName()] = expiration; - return jv; - } - - // A proposed transaction in the form the ledger stores it: unsigned, - // ticket-based, with the fee the target account will pay fixed now. - static json::Value - unsignedPayload( - jtx::Env const& env, - jtx::Account const& target, - jtx::Account const& dest, - std::uint32_t ticketSeq) - { - json::Value tx = jtx::pay(target, dest, jtx::XRP(1)); - tx[jss::Sequence] = 0; - tx[sfTicketSequence.getJsonName()] = ticketSeq; - tx[jss::Fee] = std::to_string(env.current()->fees().base.drops()); - tx[jss::SigningPubKey] = ""; - return tx; - } - void - testCreate(FeatureBitset features) + testReserveCounts() { - testcase("create proposal object"); + testcase("proposal reserve"); + + using namespace jtx; + + BEAST_EXPECT(proposal::kProposalOwnerCount == 5); + BEAST_EXPECT(proposal::kBatchProposalOwnerCount == 10); + } + + // Nothing about the transaction is available before the amendment is + // active, not even to an otherwise valid proposal. + void + testDisabled(FeatureBitset features) + { + testcase("amendment disabled"); using namespace jtx; using namespace std::chrono_literals; - Env env{*this, features}; + Env env{*this, features - featureCosign}; - Account const alice{"alice"}; // the proposer - Account const target{"target"}; // the account the proposal is for + 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 targetTicketSeq = env.seq(target) + 1; - env(ticket::create(target, 1)); + std::uint32_t const targetTicketSeq = proposal::createTicket(env, target); + + env(proposal::create( + alice, + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), targetTicketSeq), + proposal::expiration(env, 100s)), + Ter(temDISABLED)); env.close(); - // The proposed transaction is stored unsigned: no signature fields and - // an empty SigningPubKey. It is ticket-based so unrelated target account - // activity cannot invalidate it while signatures are collected. - json::Value const proposedTx = unsignedPayload(env, target, bob, targetTicketSeq); - - std::uint32_t const expiration = (env.now() + 100s).time_since_epoch().count(); - - env(proposalCreate(alice, proposedTx, expiration)); - env.close(); - - auto const sle = env.le(keylet::txProposal(target.id(), targetTicketSeq)); - BEAST_EXPECT(sle); - if (!sle) - return; - - BEAST_EXPECT(sle->getAccountID(sfOwner) == alice.id()); - BEAST_EXPECT(sle->getFieldU32(sfExpiration) == expiration); - - auto const stored = sle->getFieldObject(sfProposedTransaction); - BEAST_EXPECT(stored.getAccountID(sfAccount) == target.id()); - BEAST_EXPECT(stored.getFieldU32(sfSequence) == 0); - BEAST_EXPECT(stored.getFieldU32(sfTicketSequence) == targetTicketSeq); - BEAST_EXPECT(stored.getFieldVL(sfSigningPubKey).empty()); - - // The proposal reserves several owner increments against the proposer. - // The target only owns the Ticket used by the proposed transaction. - BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); - BEAST_EXPECT(ownerCount(env, target) == 1); + BEAST_EXPECT(!proposal::entry(env, target, targetTicketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == 0); } - // The proposed transaction must be storable in unsigned canonical form and - // must be a transaction that could be submitted on its own. Each case below - // takes an otherwise valid payload and breaks exactly one of those rules. + // The proposed transaction must be a transaction that could be submitted on + // its own. Each case below takes an otherwise valid payload and breaks + // exactly one of those rules; the rules about its signature fields are + // covered by testRejectedSignatureFields. void testRejectedPayload(FeatureBitset features) { @@ -131,49 +98,21 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite env.fund(XRP(10000), alice, target, bob); env.close(); - std::uint32_t const targetTicketSeq = env.seq(target) + 1; - env(ticket::create(target, 1)); - env.close(); + std::uint32_t const targetTicketSeq = proposal::createTicket(env, target); - std::uint32_t const expiration = (env.now() + 100s).time_since_epoch().count(); + std::uint32_t const expiration = proposal::expiration(env, 100s); // A payload that is accepted as-is; every case starts from this. - auto payload = [&]() { return unsignedPayload(env, target, bob, targetTicketSeq); }; - - auto reject = [&](json::Value const& proposedTx, TER expected) { - env(proposalCreate(alice, proposedTx, expiration), Ter(expected)); - env.close(); - BEAST_EXPECT(!env.le(keylet::txProposal(target.id(), targetTicketSeq))); - BEAST_EXPECT(ownerCount(env, alice) == 0); + auto payload = [&]() { + return proposal::unsignedPayload(env, pay(target, bob, XRP(1)), targetTicketSeq); }; - // Signatures may only ever arrive through TransactionProposalSign. - { - json::Value tx = payload(); - tx[sfTxnSignature.getJsonName()] = "DEADBEEF"; - reject(tx, temBAD_SIGNER); - } - { - json::Value tx = payload(); - auto& signer = tx[sfSigners.getJsonName()][0u][sfSigner.getJsonName()]; - signer[jss::Account] = bob.human(); - signer[jss::SigningPubKey] = strHex(bob.pk().slice()); - signer[sfTxnSignature.getJsonName()] = "DEADBEEF"; - reject(tx, temBAD_SIGNER); - } - - // SigningPubKey must be present and empty: absent is not the same as - // empty, and a set key means the payload was signed for single-signing. - { - json::Value tx = payload(); - tx.removeMember(jss::SigningPubKey); - reject(tx, temBAD_SIGNER); - } - { - json::Value tx = payload(); - tx[jss::SigningPubKey] = strHex(target.pk().slice()); - reject(tx, temBAD_SIGNER); - } + auto reject = [&](json::Value const& proposedTx, TER expected) { + env(proposal::create(alice, proposedTx, expiration), Ter(expected)); + env.close(); + BEAST_EXPECT(!proposal::entry(env, target, targetTicketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == 0); + }; // A pseudo-transaction is never submittable by an account. { @@ -196,28 +135,6 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite reject(tx, temINVALID); } - // The remaining signature containers are just as forbidden as a bare - // TxnSignature or Signers array. - { - json::Value tx = payload(); - auto& bs = tx[sfBatchSigners.getJsonName()][0u][sfBatchSigner.getJsonName()]; - bs[jss::Account] = bob.human(); - bs[jss::SigningPubKey] = strHex(bob.pk().slice()); - bs[sfTxnSignature.getJsonName()] = "DEADBEEF"; - reject(tx, temBAD_SIGNER); - } - { - json::Value tx = payload(); - tx[sfCounterpartySignature.getJsonName()][jss::SigningPubKey] = - strHex(bob.pk().slice()); - reject(tx, temBAD_SIGNER); - } - { - json::Value tx = payload(); - tx[sfSponsorSignature.getJsonName()][jss::SigningPubKey] = strHex(bob.pk().slice()); - reject(tx, temBAD_SIGNER); - } - // The proposed transaction must be ticket-based: a missing // TicketSequence, or a live Sequence alongside it, is rejected. { @@ -241,24 +158,30 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite // Expiration must be present and non-zero. { - env(proposalCreate(alice, payload(), 0), Ter(temBAD_EXPIRATION)); + env(proposal::create(alice, payload(), 0), Ter(temBAD_EXPIRATION)); env.close(); - BEAST_EXPECT(!env.le(keylet::txProposal(target.id(), targetTicketSeq))); + BEAST_EXPECT(!proposal::entry(env, target, targetTicketSeq)); BEAST_EXPECT(ownerCount(env, alice) == 0); } } - // Nothing about the transaction is available before the amendment is - // active, not even to an otherwise valid proposal. + // A proposal is stored in unsigned canonical form: an empty SigningPubKey + // and no signature field whatsoever. Signatures may only ever arrive + // through TransactionProposalSign, so a payload is rejected for carrying a + // signature container at all — whatever that container happens to hold. + // Each container below is therefore filled every way it could be, + // including combinations that could never verify: an empty container, a + // key with no signature, a signature with no key, and a signature next to + // the empty SigningPubKey the canonical form requires. void - testDisabled(FeatureBitset features) + testRejectedSignatureFields(FeatureBitset features) { - testcase("amendment disabled"); + testcase("reject payload carrying a signature"); using namespace jtx; using namespace std::chrono_literals; - Env env{*this, features - featureCosign}; + Env env{*this, features}; Account const alice{"alice"}; Account const target{"target"}; @@ -266,18 +189,159 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite env.fund(XRP(10000), alice, target, bob); env.close(); - std::uint32_t const targetTicketSeq = env.seq(target) + 1; - env(ticket::create(target, 1)); - env.close(); + std::uint32_t const targetTicketSeq = proposal::createTicket(env, target); - std::uint32_t const expiration = (env.now() + 100s).time_since_epoch().count(); + std::uint32_t const expiration = proposal::expiration(env, 100s); - env(proposalCreate(alice, unsignedPayload(env, target, bob, targetTicketSeq), expiration), - Ter(temDISABLED)); - env.close(); + std::string const key = strHex(bob.pk().slice()); + std::string const sig = "DEADBEEF"; - BEAST_EXPECT(!env.le(keylet::txProposal(target.id(), targetTicketSeq))); - BEAST_EXPECT(ownerCount(env, alice) == 0); + // The payloads every case starts from, each accepted as-is. Two cases + // below would otherwise build the same payload, and the same proposal + // cannot be submitted twice — the second is turned away as a duplicate + // rather than judged again — so each call pays a different amount. + // Nothing here turns on the amount. + std::uint32_t paid = 0; + auto payment = [&]() { + return proposal::unsignedPayload(env, pay(target, bob, drops(++paid)), targetTicketSeq); + }; + auto sponsoredPayment = [&]() { + json::Value tx = pay(target, bob, drops(++paid)); + tx[sfSponsor.getJsonName()] = alice.human(); + tx[sfSponsorFlags.getJsonName()] = spfSponsorFee; + return proposal::unsignedPayload(env, tx, targetTicketSeq); + }; + auto loanSet = [&]() { + json::Value tx = loan::set(target, uint256{1}, 1'000 + ++paid); + tx[sfCounterparty.getJsonName()] = bob.human(); + return proposal::unsignedPayload(env, tx, targetTicketSeq); + }; + auto batchTx = [&]() { + return proposal::unsignedBatch( + env, + target, + targetTicketSeq, + tfAllOrNothing, + {proposal::innerTx(pay(target, bob, drops(++paid)), env.seq(target)), + proposal::innerTx(pay(target, bob, drops(++paid)), env.seq(target) + 1)}); + }; + + auto reject = [&](json::Value const& proposedTx) { + env(proposal::create(alice, proposedTx, expiration), Ter(temBAD_SIGNER)); + env.close(); + BEAST_EXPECT(!proposal::entry(env, target, targetTicketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == 0); + }; + + // Every way of filling in a signature. The payload's own signature + // fields and a co-signature object hold the same three members, so the + // same fills apply to both. + std::vector> const fills{ + [&](json::Value& o) { o[jss::SigningPubKey] = key; }, + [&](json::Value& o) { o[sfTxnSignature.getJsonName()] = sig; }, + [&](json::Value& o) { + o[jss::SigningPubKey] = ""; + o[sfTxnSignature.getJsonName()] = sig; + }, + // Signed the ordinary way, which is the likeliest way one of these + // arrives here. + [&](json::Value& o) { + o[jss::SigningPubKey] = key; + o[sfTxnSignature.getJsonName()] = sig; + }, + // Multi-signed: the signer's own key is empty and the signatures + // sit in a nested Signers array. Each entry needs all three of + // Account, SigningPubKey and TxnSignature to parse at all, so only + // their values can vary. + [&](json::Value& o) { + o[jss::SigningPubKey] = ""; + auto& signer = o[sfSigners.getJsonName()][0u][sfSigner.getJsonName()]; + signer[jss::Account] = bob.human(); + signer[jss::SigningPubKey] = key; + signer[sfTxnSignature.getJsonName()] = sig; + }, + [&](json::Value& o) { + o[jss::SigningPubKey] = ""; + auto& signer = o[sfSigners.getJsonName()][0u][sfSigner.getJsonName()]; + signer[jss::Account] = bob.human(); + signer[jss::SigningPubKey] = ""; + signer[sfTxnSignature.getJsonName()] = sig; + }, + }; + + // Every place a signature could sit, on a payload of a type that + // carries it: a Counterparty's signature belongs to a LoanSet and + // BatchSigners to a Batch, while a Sponsor's signature and the + // payload's own signature fields sit on any transaction. A signature + // is no more storable for being a field its transaction type expects + // (On-Chain Cosigner spec §6.1, §6.6.3). + struct Place + { + std::function payload; + std::function at; + }; + + std::vector const places{ + {payment, [](json::Value& tx) -> json::Value& { return tx; }}, + {loanSet, + [](json::Value& tx) -> json::Value& { + auto& o = tx[sfCounterpartySignature.getJsonName()]; + o = json::Value{json::ValueType::Object}; + return o; + }}, + {sponsoredPayment, + [](json::Value& tx) -> json::Value& { + auto& o = tx[sfSponsorSignature.getJsonName()]; + o = json::Value{json::ValueType::Object}; + return o; + }}, + // A BatchSigners entry names the account it speaks for; the other + // two co-signatures are fixed by the transaction they belong to and + // do not. + {batchTx, + [&](json::Value& tx) -> json::Value& { + auto& o = tx[sfBatchSigners.getJsonName()][0u][sfBatchSigner.getJsonName()]; + o[jss::Account] = bob.human(); + return o; + }}, + }; + + for (auto const& place : places) + { + for (auto const& fill : fills) + { + json::Value tx = place.payload(); + fill(place.at(tx)); + reject(tx); + } + } + + // Every place but the payload itself: a co-signature object is + // disqualifying by its presence alone, so each is rejected left empty + // too. The payload's own fields have no such case — left alone they are + // the canonical form. + for (std::size_t i = 1; i < places.size(); ++i) + { + json::Value tx = places[i].payload(); + places[i].at(tx); + reject(tx); + } + + // Nor does the payload have a counterpart for an absent SigningPubKey: + // in a co-signature object an absent member is just an unfilled one, + // but at the top level it is not the same as an empty one, with or + // without a signature beside it. + { + json::Value tx = payment(); + tx.removeMember(jss::SigningPubKey); + reject(tx); + } + { + json::Value tx = payment(); + tx.removeMember(jss::SigningPubKey); + tx[sfTxnSignature.getJsonName()] = sig; + reject(tx); + } } // A proposal that could never be completed must not be stored, and a @@ -299,30 +363,31 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite env.fund(XRP(10000), alice, target, bob); env.close(); - std::uint32_t const firstTicketSeq = env.seq(target) + 1; - env(ticket::create(target, 3)); - env.close(); + std::uint32_t const firstTicketSeq = proposal::createTicket(env, target, 3); - std::uint32_t const expiration = (env.now() + 100s).time_since_epoch().count(); + std::uint32_t const expiration = proposal::expiration(env, 100s); + + auto payload = [&](std::uint32_t ticketSeq) { + return proposal::unsignedPayload(env, pay(target, bob, XRP(1)), ticketSeq); + }; // The proposal's own expiration has already passed. { - std::uint32_t const past = env.now().time_since_epoch().count(); - env(proposalCreate(alice, unsignedPayload(env, target, bob, firstTicketSeq), past), + env(proposal::create(alice, payload(firstTicketSeq), proposal::expiration(env, 0s)), Ter(tecEXPIRED)); env.close(); - BEAST_EXPECT(!env.le(keylet::txProposal(target.id(), firstTicketSeq))); + BEAST_EXPECT(!proposal::entry(env, target, firstTicketSeq)); BEAST_EXPECT(ownerCount(env, alice) == 0); } // The proposed transaction's own ledger bound has passed: the ordinary // path would reject it with tefMAX_LEDGER, so it can never complete. { - json::Value tx = unsignedPayload(env, target, bob, firstTicketSeq); + json::Value tx = payload(firstTicketSeq); tx[sfLastLedgerSequence.getJsonName()] = env.current()->seq() - 1; - env(proposalCreate(alice, tx, expiration), Ter(tecEXPIRED)); + env(proposal::create(alice, tx, expiration), Ter(tecEXPIRED)); env.close(); - BEAST_EXPECT(!env.le(keylet::txProposal(target.id(), firstTicketSeq))); + BEAST_EXPECT(!proposal::entry(env, target, firstTicketSeq)); BEAST_EXPECT(ownerCount(env, alice) == 0); } @@ -331,213 +396,49 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite // passes, so it is rejected the same as one already in the past // (On-Chain Cosigner spec §5.3.2.2). { - json::Value tx = unsignedPayload(env, target, bob, firstTicketSeq); + json::Value tx = payload(firstTicketSeq); tx[sfLastLedgerSequence.getJsonName()] = env.current()->seq(); - env(proposalCreate(alice, tx, expiration), Ter(tecEXPIRED)); + env(proposal::create(alice, tx, expiration), Ter(tecEXPIRED)); env.close(); - BEAST_EXPECT(!env.le(keylet::txProposal(target.id(), firstTicketSeq))); + BEAST_EXPECT(!proposal::entry(env, target, firstTicketSeq)); BEAST_EXPECT(ownerCount(env, alice) == 0); } // With no ledger bound on the proposed transaction, the proposal is // created normally. { - env(proposalCreate( - alice, unsignedPayload(env, target, bob, firstTicketSeq), expiration)); + env(proposal::create(alice, payload(firstTicketSeq), expiration)); env.close(); - BEAST_EXPECT(env.le(keylet::txProposal(target.id(), firstTicketSeq))); + BEAST_EXPECT(proposal::entry(env, target, firstTicketSeq)); BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); } // The target and ticket already carry a proposal. { - env(proposalCreate( - alice, unsignedPayload(env, target, bob, firstTicketSeq), expiration), - Ter(tecDUPLICATE)); + env(proposal::create(alice, payload(firstTicketSeq), expiration), Ter(tecDUPLICATE)); env.close(); BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); } // A different ticket of the same target is a different proposal. { - env(proposalCreate( - alice, unsignedPayload(env, target, bob, firstTicketSeq + 1), expiration)); + env(proposal::create(alice, payload(firstTicketSeq + 1), expiration)); env.close(); - BEAST_EXPECT(env.le(keylet::txProposal(target.id(), firstTicketSeq + 1))); + BEAST_EXPECT(proposal::entry(env, target, firstTicketSeq + 1)); BEAST_EXPECT(ownerCount(env, alice) == 2 * proposal::kProposalOwnerCount); } // The target account does not exist, so it can never sign. { - env(proposalCreate(alice, unsignedPayload(env, carol, bob, 1), expiration), + env(proposal::create( + alice, proposal::unsignedPayload(env, pay(carol, bob, XRP(1)), 1), expiration), Ter(tecNO_TARGET)); env.close(); - BEAST_EXPECT(!env.le(keylet::txProposal(carol.id(), 1))); + BEAST_EXPECT(!proposal::entry(env, carol, 1)); BEAST_EXPECT(ownerCount(env, alice) == 2 * proposal::kProposalOwnerCount); } } - // The proposer holds the proposal's reserve until it is resolved. - void - testReserve(FeatureBitset features) - { - testcase("proposer 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), target, bob); - env.close(); - - std::uint32_t const targetTicketSeq = env.seq(target) + 1; - env(ticket::create(target, 1)); - env.close(); - - // Fund alice just short of the reserve the proposal requires. - env.fund( - env.current()->fees().accountReserve(proposal::kProposalOwnerCount, 1) - drops(1), - alice); - env.close(); - - std::uint32_t const expiration = (env.now() + 100s).time_since_epoch().count(); - json::Value const proposedTx = unsignedPayload(env, target, bob, targetTicketSeq); - - env(proposalCreate(alice, proposedTx, expiration), Ter(tecINSUFFICIENT_RESERVE)); - env.close(); - BEAST_EXPECT(!env.le(keylet::txProposal(target.id(), targetTicketSeq))); - BEAST_EXPECT(ownerCount(env, alice) == 0); - - env(pay(bob, alice, XRP(10))); - env.close(); - - env(proposalCreate(alice, proposedTx, expiration)); - env.close(); - BEAST_EXPECT(env.le(keylet::txProposal(target.id(), targetTicketSeq))); - BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); - } - - // A proposed Batch holds several inner transactions and the signatures of - // every account they touch, so it reserves more than an ordinary proposal. - void - testBatchReserve(FeatureBitset features) - { - testcase("proposed batch 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(); - - std::uint32_t const targetTicketSeq = env.seq(target) + 1; - env(ticket::create(target, 1)); - env.close(); - - auto inner = [&](std::uint32_t seq) { - json::Value tx = pay(target, bob, XRP(1)); - tx[jss::Sequence] = seq; - tx[jss::Fee] = "0"; - tx[jss::Flags] = tfInnerBatchTxn; - tx[jss::SigningPubKey] = ""; - return tx; - }; - - json::Value proposedTx; - proposedTx[jss::TransactionType] = jss::Batch; - proposedTx[jss::Account] = target.human(); - proposedTx[jss::Flags] = tfAllOrNothing; - proposedTx[jss::Sequence] = 0; - proposedTx[sfTicketSequence.getJsonName()] = targetTicketSeq; - proposedTx[jss::Fee] = std::to_string(batch::calcBatchFee(env, 0, 2).drops()); - proposedTx[jss::SigningPubKey] = ""; - proposedTx[jss::RawTransactions][0u][jss::RawTransaction] = inner(env.seq(target)); - proposedTx[jss::RawTransactions][1u][jss::RawTransaction] = inner(env.seq(target) + 1); - - std::uint32_t const expiration = (env.now() + 100s).time_since_epoch().count(); - - env(proposalCreate(alice, proposedTx, expiration)); - env.close(); - - BEAST_EXPECT(env.le(keylet::txProposal(target.id(), targetTicketSeq))); - BEAST_EXPECT(ownerCount(env, alice) == proposal::kBatchProposalOwnerCount); - } - - // A multi-account Batch is the primary motivating case (On-Chain Cosigner spec §10): its inner - // transactions touch accounts other than the outer one, so submitting it - // directly would require a BatchSigners entry per participant. A proposal is - // stored unsigned, so those signatures are collected on-ledger afterward and - // the signer-presence match is skipped at creation time (On-Chain Cosigner spec §5.3.1.2). - void - testMultiAccountBatch(FeatureBitset features) - { - testcase("proposed multi-account batch"); - - using namespace jtx; - using namespace std::chrono_literals; - - Env env{*this, features}; - - Account const alice{"alice"}; - Account const target{"target"}; // outer account of the batch - Account const bob{"bob"}; // a distinct inner participant - env.fund(XRP(10000), alice, target, bob); - env.close(); - - std::uint32_t const targetTicketSeq = env.seq(target) + 1; - env(ticket::create(target, 1)); - env.close(); - - auto inner = [&](Account const& from, Account const& to, std::uint32_t seq) { - json::Value tx = pay(from, to, XRP(1)); - tx[jss::Sequence] = seq; - tx[jss::Fee] = "0"; - tx[jss::Flags] = tfInnerBatchTxn; - tx[jss::SigningPubKey] = ""; - return tx; - }; - - // One inner from the outer account, one from bob: bob is a required - // signer, so a direct submission would need his BatchSigners entry. - json::Value proposedTx; - proposedTx[jss::TransactionType] = jss::Batch; - proposedTx[jss::Account] = target.human(); - proposedTx[jss::Flags] = tfAllOrNothing; - proposedTx[jss::Sequence] = 0; - proposedTx[sfTicketSequence.getJsonName()] = targetTicketSeq; - proposedTx[jss::Fee] = std::to_string(batch::calcBatchFee(env, 1, 2).drops()); - proposedTx[jss::SigningPubKey] = ""; - proposedTx[jss::RawTransactions][0u][jss::RawTransaction] = - inner(target, bob, env.seq(target)); - proposedTx[jss::RawTransactions][1u][jss::RawTransaction] = - inner(bob, target, env.seq(bob)); - - std::uint32_t const expiration = (env.now() + 100s).time_since_epoch().count(); - - env(proposalCreate(alice, proposedTx, expiration)); - env.close(); - - auto const sle = env.le(keylet::txProposal(target.id(), targetTicketSeq)); - BEAST_EXPECT(sle); - if (!sle) - return; - - // The proposal is stored without any BatchSigners: the participants' - // signatures are collected later through TransactionProposalSign. - auto const stored = sle->getFieldObject(sfProposedTransaction); - BEAST_EXPECT(!stored.isFieldPresent(sfBatchSigners)); - BEAST_EXPECT(ownerCount(env, alice) == proposal::kBatchProposalOwnerCount); - } - // The target account must be able to authorize a transaction through a // SignerList, so a pseudo-account (here an AMM's) cannot be a target even // though it exists on-ledger (On-Chain Cosigner spec §5.3.2.5). @@ -568,21 +469,117 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite env.close(); // A well-formed Payment whose target is the AMM's pseudo-account. - json::Value proposedTx = pay(alice, bob, XRP(1)); - proposedTx[jss::Account] = toBase58(amm.ammAccount()); - proposedTx[jss::Sequence] = 0; - proposedTx[sfTicketSequence.getJsonName()] = 1; - proposedTx[jss::Fee] = std::to_string(env.current()->fees().base.drops()); - proposedTx[jss::SigningPubKey] = ""; + json::Value tx = pay(alice, bob, XRP(1)); + tx[jss::Account] = toBase58(amm.ammAccount()); + json::Value const proposedTx = proposal::unsignedPayload(env, tx, 1); - std::uint32_t const expiration = (env.now() + 100s).time_since_epoch().count(); - - env(proposalCreate(proposer, proposedTx, expiration), Ter(tecNO_PERMISSION)); + env(proposal::create(proposer, proposedTx, proposal::expiration(env, 100s)), + Ter(tecNO_PERMISSION)); env.close(); - BEAST_EXPECT(!env.le(keylet::txProposal(amm.ammAccount(), 1))); + BEAST_EXPECT(!proposal::entry(env, amm.ammAccount(), 1)); BEAST_EXPECT(ownerCount(env, proposer) == 0); } + void + testCreate(FeatureBitset features) + { + testcase("create proposal object"); + + 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 targetTicketSeq = proposal::createTicket(env, target); + + // The proposed transaction is stored unsigned: no signature fields and + // an empty SigningPubKey. It is ticket-based so unrelated target account + // activity cannot invalidate it while signatures are collected. + json::Value const proposedTx = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), targetTicketSeq); + + std::uint32_t const expiration = proposal::expiration(env, 100s); + + env(proposal::create(alice, proposedTx, expiration)); + env.close(); + + auto const sle = proposal::entry(env, target, targetTicketSeq); + if (!BEAST_EXPECT(sle)) + return; + + BEAST_EXPECT(sle->getAccountID(sfOwner) == alice.id()); + BEAST_EXPECT(sle->getFieldU32(sfExpiration) == expiration); + + auto const stored = sle->getFieldObject(sfProposedTransaction); + BEAST_EXPECT(stored.getAccountID(sfAccount) == target.id()); + BEAST_EXPECT(stored.getFieldU32(sfSequence) == 0); + BEAST_EXPECT(stored.getFieldU32(sfTicketSequence) == targetTicketSeq); + BEAST_EXPECT(stored.getFieldVL(sfSigningPubKey).empty()); + + // The proposal reserves several owner increments against the proposer. + // The target only owns the Ticket used by the proposed transaction. + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + BEAST_EXPECT(ownerCount(env, target) == 1); + } + + // A proposal carries a transaction of any type: what the proposal requires + // of the payload — unsigned, ticket-based, fee fixed — is independent of + // the transaction being proposed, so anything a target account's signer + // list could authorize can be proposed for it. + void + testOtherTransactionTypes(FeatureBitset features) + { + testcase("proposals for other transaction types"); + + 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 proposals are for + Account const bob{"bob"}; + Account const gw{"gw"}; + // NOLINTNEXTLINE(readability-identifier-naming) + auto const USD = gw["USD"]; + env.fund(XRP(10000), alice, target, bob, gw); + env.close(); + + // One payload per transaction type, each straight from the generator + // the ordinary tests for that type use. + std::vector const payloads{ + noop(target), // AccountSet + offer(target, USD(1), XRP(1)), // OfferCreate + trust(target, USD(1000)), // TrustSet + signers(target, 1, {{bob, 1}}), // SignerListSet + deposit::auth(target, bob), // DepositPreauth + token::mint(target, 0), // NFTokenMint + }; + + // A proposal is keyed by target and ticket, so each payload needs its + // own ticket. + std::uint32_t const firstTicketSeq = + proposal::createTicket(env, target, static_cast(payloads.size())); + std::uint32_t const expiration = proposal::expiration(env, 100s); + + for (std::size_t i = 0; i < payloads.size(); ++i) + { + std::uint32_t const ticketSeq = firstTicketSeq + static_cast(i); + env(proposal::create( + alice, proposal::unsignedPayload(env, payloads[i], ticketSeq), expiration)); + env.close(); + BEAST_EXPECT(proposal::entry(env, target, ticketSeq)); + } + + BEAST_EXPECT(ownerCount(env, alice) == payloads.size() * proposal::kProposalOwnerCount); + } + // A proposed transaction may itself require an auxiliary co-signer beyond // its own Account: a LoanSet's Counterparty, or the Sponsor of an // account-level SponsorshipTransfer (On-Chain Cosigner spec §6.1, §6.6.3). That co-signature @@ -605,60 +602,188 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite env.fund(XRP(10000), alice, borrower); env.close(); - std::uint32_t const expiration = (env.now() + 100s).time_since_epoch().count(); + std::uint32_t const expiration = proposal::expiration(env, 100s); // LoanSet: the Counterparty's signature is collected later; it must // not be required up front. { - std::uint32_t const ticketSeq = env.seq(borrower) + 1; - env(ticket::create(borrower, 1)); - env.close(); + std::uint32_t const ticketSeq = proposal::createTicket(env, borrower); - json::Value tx = loan::set(borrower, uint256{1}, 1'000); - tx[jss::Sequence] = 0; - tx[sfTicketSequence.getJsonName()] = ticketSeq; - tx[jss::Fee] = std::to_string(env.current()->fees().base.drops()); - tx[jss::SigningPubKey] = ""; + json::Value const tx = + proposal::unsignedPayload(env, loan::set(borrower, uint256{1}, 1'000), ticketSeq); - env(proposalCreate(alice, tx, expiration)); + env(proposal::create(alice, tx, expiration)); env.close(); - BEAST_EXPECT(env.le(keylet::txProposal(borrower.id(), ticketSeq))); + BEAST_EXPECT(proposal::entry(env, borrower, ticketSeq)); } // SponsorshipTransfer (account-level reserve sponsorship): the // Sponsor's signature is likewise collected later. { - std::uint32_t const ticketSeq = env.seq(borrower) + 1; - env(ticket::create(borrower, 1)); - env.close(); + std::uint32_t const ticketSeq = proposal::createTicket(env, borrower); json::Value tx = sponsor::transfer(borrower, tfSponsorshipCreate); tx[sfSponsor.getJsonName()] = alice.human(); tx[sfSponsorFlags.getJsonName()] = spfSponsorReserve; - tx[jss::Sequence] = 0; - tx[sfTicketSequence.getJsonName()] = ticketSeq; - tx[jss::Fee] = std::to_string(env.current()->fees().base.drops()); - tx[jss::SigningPubKey] = ""; - env(proposalCreate(alice, tx, expiration)); + env(proposal::create(alice, proposal::unsignedPayload(env, tx, ticketSeq), expiration)); env.close(); - BEAST_EXPECT(env.le(keylet::txProposal(borrower.id(), ticketSeq))); + BEAST_EXPECT(proposal::entry(env, borrower, ticketSeq)); } } + // The proposer holds the proposal's reserve until it is resolved. + void + testReserve(FeatureBitset features) + { + testcase("proposer 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), target, bob); + env.close(); + + std::uint32_t const targetTicketSeq = proposal::createTicket(env, target); + + // Fund alice just short of the reserve the proposal requires. + env.fund( + env.current()->fees().accountReserve(proposal::kProposalOwnerCount, 1) - drops(1), + alice); + env.close(); + + std::uint32_t const expiration = proposal::expiration(env, 100s); + json::Value const proposedTx = + proposal::unsignedPayload(env, pay(target, bob, XRP(1)), targetTicketSeq); + + env(proposal::create(alice, proposedTx, expiration), Ter(tecINSUFFICIENT_RESERVE)); + env.close(); + BEAST_EXPECT(!proposal::entry(env, target, targetTicketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == 0); + + env(pay(bob, alice, XRP(10))); + env.close(); + + env(proposal::create(alice, proposedTx, expiration)); + env.close(); + BEAST_EXPECT(proposal::entry(env, target, targetTicketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kProposalOwnerCount); + } + + // A proposed Batch holds several inner transactions and the signatures of + // every account they touch, so it reserves more than an ordinary proposal. + void + testBatchReserve(FeatureBitset features) + { + testcase("proposed batch 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(); + + std::uint32_t const targetTicketSeq = proposal::createTicket(env, target); + + // Both inner transactions are the outer account's own, so no further + // signatures will be collected for them. + json::Value const proposedTx = proposal::unsignedBatch( + env, + target, + targetTicketSeq, + tfAllOrNothing, + {proposal::innerTx(pay(target, bob, XRP(1)), env.seq(target)), + proposal::innerTx(pay(target, bob, XRP(1)), env.seq(target) + 1)}); + + env(proposal::create(alice, proposedTx, proposal::expiration(env, 100s))); + env.close(); + + BEAST_EXPECT(proposal::entry(env, target, targetTicketSeq)); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kBatchProposalOwnerCount); + } + + // A multi-account Batch is the primary motivating case (On-Chain Cosigner spec §10): its inner + // transactions touch accounts other than the outer one, so submitting it + // directly would require a BatchSigners entry per participant. A proposal is + // stored unsigned, so those signatures are collected on-ledger afterward and + // the signer-presence match is skipped at creation time (On-Chain Cosigner spec §5.3.1.2). + void + testMultiAccountBatch(FeatureBitset features) + { + testcase("proposed multi-account batch"); + + using namespace jtx; + using namespace std::chrono_literals; + + Env env{*this, features}; + + Account const alice{"alice"}; + Account const target{"target"}; // outer account of the batch + Account const bob{"bob"}; // a distinct inner participant + env.fund(XRP(10000), alice, target, bob); + env.close(); + + std::uint32_t const targetTicketSeq = proposal::createTicket(env, target); + + // One inner from the outer account, one from bob: bob is a required + // signer, so a direct submission would need his BatchSigners entry. + json::Value const proposedTx = proposal::unsignedBatch( + env, + target, + targetTicketSeq, + tfAllOrNothing, + {proposal::innerTx(pay(target, bob, XRP(1)), env.seq(target)), + proposal::innerTx(pay(bob, target, XRP(1)), env.seq(bob))}); + + env(proposal::create(alice, proposedTx, proposal::expiration(env, 100s))); + env.close(); + + auto const sle = proposal::entry(env, target, targetTicketSeq); + if (!BEAST_EXPECT(sle)) + return; + + // The proposal is stored without any BatchSigners: the participants' + // signatures are collected later through TransactionProposalSign. + auto const stored = sle->getFieldObject(sfProposedTransaction); + BEAST_EXPECT(!stored.isFieldPresent(sfBatchSigners)); + BEAST_EXPECT(ownerCount(env, alice) == proposal::kBatchProposalOwnerCount); + } + void run() override { using namespace jtx; - testDisabled(testableAmendments()); - testCreate(testableAmendments()); - testRejectedPayload(testableAmendments()); - testPreclaim(testableAmendments()); - testReserve(testableAmendments()); - testBatchReserve(testableAmendments()); - testMultiAccountBatch(testableAmendments()); - testPseudoTarget(testableAmendments()); - testAuxiliaryCoSignatureTypes(testableAmendments()); + + FeatureBitset const all{testableAmendments()}; + + testReserveCounts(); + + // Preflight + testDisabled(all); + testRejectedPayload(all); + testRejectedSignatureFields(all); + + // Preclaim + testPreclaim(all); + testPseudoTarget(all); + + // Apply + testCreate(all); + testOtherTransactionTypes(all); + testAuxiliaryCoSignatureTypes(all); + testReserve(all); + testBatchReserve(all); + testMultiAccountBatch(all); } }; diff --git a/src/test/jtx/impl/proposal.cpp b/src/test/jtx/impl/proposal.cpp new file mode 100644 index 0000000000..b9d7c69a27 --- /dev/null +++ b/src/test/jtx/impl/proposal.cpp @@ -0,0 +1,127 @@ +#include + +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include +#include +#include +#include + +namespace xrpl::test::jtx::proposal { + +json::Value +create(Account const& proposer, json::Value const& proposedTx, std::uint32_t expiration) +{ + json::Value jv; + jv[jss::TransactionType] = "TransactionProposalCreate"; + jv[jss::Account] = proposer.human(); + jv[sfProposedTransaction.jsonName] = proposedTx; + jv[sfExpiration.jsonName] = expiration; + return jv; +} + +json::Value +unsignedPayload(Env const& env, json::Value tx, std::uint32_t ticketSeq) +{ + // Unsigned canonical form: an empty SigningPubKey and no signature fields + // at all. Signatures may only ever arrive through TransactionProposalSign. + tx[jss::SigningPubKey] = ""; + + // Ticket-based rather than sequence-based. Sequence is a required common + // field, so "no Sequence" is expressed as a Sequence of 0. + tx[jss::Sequence] = 0; + tx[sfTicketSequence.jsonName] = ticketSeq; + + // The target account pays this fee when the completed transaction is + // submitted, so it is fixed now. A fee already chosen by the caller stands. + fillFee(tx, *env.current()); + + return tx; +} + +json::Value +innerTx(json::Value tx, std::uint32_t seq) +{ + return batch::Inner{std::move(tx), seq}.getTxn(); +} + +json::Value +unsignedBatch( + Env const& env, + Account const& target, + std::uint32_t ticketSeq, + std::uint32_t flags, + std::vector const& inners, + std::optional numSigners) +{ + // Each inner account other than the outer one will contribute one + // BatchSigners entry once the signatures are collected, and the outer fee + // has to cover them from the start (Batch::calculateBaseFee). + std::uint32_t const signers = numSigners ? *numSigners : [&]() { + std::set participants; + for (auto const& inner : inners) + { + if (auto const account = inner[jss::Account].asString(); account != target.human()) + participants.insert(account); + } + return static_cast(participants.size()); + }(); + + json::Value jv = batch::outer( + target, + 0, + batch::calcBatchFee(env, signers, static_cast(inners.size())), + flags); + + json::Value& rawTransactions = jv[jss::RawTransactions]; + for (auto const& inner : inners) + rawTransactions[rawTransactions.size()][jss::RawTransaction] = inner; + + return unsignedPayload(env, std::move(jv), ticketSeq); +} + +std::uint32_t +createTicket(Env& env, Account const& account, std::uint32_t count) +{ + // The tickets a TicketCreate makes are numbered from the sequence that + // follows the one it consumes. + std::uint32_t const firstTicketSeq = env.seq(account) + 1; + env(ticket::create(account, count)); + env.close(); + return firstTicketSeq; +} + +std::uint32_t +expiration(Env& env, NetClock::duration delta) +{ + return (env.now() + delta).time_since_epoch().count(); +} + +SLE::const_pointer +entry(Env const& env, AccountID const& target, std::uint32_t ticketSeq) +{ + return env.le(keylet::txProposal(target, ticketSeq)); +} + +SLE::const_pointer +entry(Env const& env, Account const& target, std::uint32_t ticketSeq) +{ + return entry(env, target.id(), ticketSeq); +} + +} // namespace xrpl::test::jtx::proposal diff --git a/src/test/jtx/proposal.h b/src/test/jtx/proposal.h new file mode 100644 index 0000000000..7482912feb --- /dev/null +++ b/src/test/jtx/proposal.h @@ -0,0 +1,133 @@ +#pragma once + +#include +#include + +#include +#include +#include +#include +#include + +#include +#include +#include + +/** + * @brief Helpers for constructing TransactionProposal test transactions. + */ +namespace xrpl::test::jtx::proposal { + +// The owner-reserve increments a proposal holds against its proposer. Tests +// spend these rather than repeating their values, so they follow the transactor +// instead of checking it; TransactionProposalCreate_test pins the values +// themselves, so a change to them has to be a deliberate one. +using xrpl::proposal::kBatchProposalOwnerCount; +using xrpl::proposal::kProposalOwnerCount; + +/** + * @brief Build a TransactionProposalCreate carrying an unsigned proposed + * transaction. + * + * @param proposer The account creating and paying the reserve for the proposal. + * @param proposedTx The proposed transaction, in unsigned canonical form; see + * unsignedPayload(). + * @param expiration Absolute time after which the proposal may no longer be + * completed. + * @return The TransactionProposalCreate JSON object. + */ +json::Value +create(Account const& proposer, json::Value const& proposedTx, std::uint32_t expiration); + +/** + * @brief Put a transaction of any type into the form a proposal stores it in: + * unsigned and ticket-based, with the fee the target account will pay fixed + * now. + * + * This takes whatever any jtx generator produces — @c pay(), @c loan::set(), + * @c sponsor::transfer(), an outer @c Batch — and applies only what the + * proposal itself demands of the payload, so a test never hand-rolls those + * fields. It leaves the rest of @p tx untouched, which is what lets a test + * build one valid payload and then break exactly one rule of it. + * + * The payload is ticket-based so unrelated activity on the target account + * cannot invalidate it while signatures are collected. A Fee already set on + * @p tx is left alone, so a payload whose fee is not the base fee — a Batch, + * say — can carry its own; otherwise the fee is filled in the same way as for + * an ordinary submission. + * + * @param env The test environment providing ledger fee settings. + * @param tx The transaction to propose. + * @param ticketSeq A ticket sequence owned by @p tx's account. + * @return The proposed transaction JSON object. + */ +json::Value +unsignedPayload(Env const& env, json::Value tx, std::uint32_t ticketSeq); + +/** + * @brief Put a transaction into the form an inner transaction of a proposed + * Batch takes, as @c batch::Inner does for an ordinary Batch. + * + * @param tx The transaction to nest. + * @param seq The sequence number of @p tx's own account. + * @return The inner transaction JSON object. + */ +json::Value +innerTx(json::Value tx, std::uint32_t seq); + +/** + * @brief An unsigned outer Batch payload holding @p inners. + * + * A proposed Batch stores no BatchSigners — the participants' signatures are + * collected on-ledger afterwards — but its fee is fixed now and must already + * cover them. By default one signer is assumed for each inner account other + * than @p target, which is what those participants will contribute. + * + * @param env The test environment providing ledger fee settings. + * @param target The outer account of the Batch, and the proposal's target. + * @param ticketSeq A ticket sequence owned by @p target. + * @param flags The Batch mode flags, e.g. @c tfAllOrNothing. + * @param inners The inner transactions; see innerTx(). + * @param numSigners Overrides the number of signatures the fee accounts for. + * @return The proposed Batch JSON object. + */ +json::Value +unsignedBatch( + Env const& env, + Account const& target, + std::uint32_t ticketSeq, + std::uint32_t flags, + std::vector const& inners, + std::optional numSigners = std::nullopt); + +/** + * @brief Create tickets for a proposal to be built against, and close the + * ledger. + * + * @param env The test environment. + * @param account The account that will own the tickets, i.e. the target of the + * proposals to come. + * @param count How many tickets to create. + * @return The first ticket sequence created; the rest follow it. + */ +std::uint32_t +createTicket(Env& env, Account const& account, std::uint32_t count = 1); + +/** + * @brief An absolute expiration @p delta past the environment's current time. + * + * @c expiration(env, 0s) is an expiration that has already passed. + */ +std::uint32_t +expiration(Env& env, NetClock::duration delta); + +/** + * @brief The proposal stored against a target account's ticket. + * @return empty if no such proposal exists. + */ +[[nodiscard]] SLE::const_pointer +entry(Env const& env, AccountID const& target, std::uint32_t ticketSeq); +[[nodiscard]] SLE::const_pointer +entry(Env const& env, Account const& target, std::uint32_t ticketSeq); + +} // namespace xrpl::test::jtx::proposal