ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-09-29 08:18:01 +00:00
Code Issues Packages Projects Releases Wiki Activity

TransactionProposalCreate edge cases and tests (#7958)

Browse Source
This commit is contained in:
Kassaking7
2026-08-24 10:49:21 -04:00
committed by GitHub
parent 0810163743
commit 5b981dc7fb
2 changed files with 178 additions and 85 deletions
Download Patch File Download Diff File Expand all files Collapse all files

183
src/test/app/TransactionProposalCreate_test.cpp
View File

@@ -14,6 +14,7 @@
#include <test/jtx/sig.h>
#include <test/jtx/sponsor.h>
#include <test/jtx/ter.h>
#include <test/jtx/ticket.h>
#include <test/jtx/token.h>
#include <test/jtx/trust.h>
@@ -122,29 +123,49 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite
BEAST_EXPECT(ownerCount(env, target) == 1);
};
// A pseudo-transaction is never submittable by an account.
// An unrecognized TransactionType cannot even be constructed as an
// STTx (there is no format to validate it against), so it is
// rejected the same as any other malformed payload.
{
json::Value tx = payload();
tx[jss::TransactionType] = 65535;
reject(tx, temMALFORMED);
}
// A pseudo-transaction is never submittable by an account. This
// payload also carries Payment-shaped fields (Amount, Destination)
// that aren't part of EnableAmendment's own template, so it fails
// STTx construction before ever reaching our own isPseudoTx check.
{
json::Value tx = payload();
tx[jss::TransactionType] = jss::EnableAmendment;
reject(tx, temINVALID);
reject(tx, temMALFORMED);
}
// An inner batch transaction bypasses the ordinary signature checks.
// The proposed transaction's own preflight rejects a standalone
// tfInnerBatchTxn (no enclosing Batch, no parentBatchId) with its own
// more specific code before reaching our own tfInnerBatchTxn check.
{
json::Value tx = payload();
tx[jss::Flags] = tfInnerBatchTxn;
reject(tx, temINVALID);
reject(tx, temINVALID_INNER_BATCH);
}
// Proposals do not nest.
// 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.
{
json::Value tx = payload();
tx[jss::TransactionType] = "TransactionProposalCreate";
reject(tx, temINVALID);
reject(tx, temMALFORMED);
}
// Nor may a proposed Batch smuggle a nested proposal in as one of its
// own inner transactions.
// own inner transactions. A second, ordinary inner transaction rides
// along only to satisfy Batch's own minimum of two inner
// transactions; it isn't itself the point of this case.
{
json::Value const nestedProposal =
proposal::create(target, payload(), proposal::expiration(env, 100s));
@@ -153,7 +174,8 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite
target,
targetTicketSeq,
tfAllOrNothing,
{proposal::innerTx(nestedProposal, env.seq(target))});
{proposal::innerTx(nestedProposal, env.seq(target)),
proposal::innerTx(pay(target, bob, XRP(1)), env.seq(target) + 1)});
reject(tx, temINVALID);
}
@@ -170,6 +192,32 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite
reject(tx, temSEQ_AND_TICKET);
}
// If this TransactionProposalCreate itself pays with a Ticket, and the
// proposed transaction targets that same account and Ticket, applying
// this transaction consumes the Ticket the proposal depends on before
// the proposal is even stored: it would be dead on arrival. target is
// proposing for itself here, so this is the Ticket it is about to pay
// with and the Ticket its own proposed payload names.
{
std::uint32_t const selfTicketSeq = proposal::createTicket(env, target);
json::Value const tx =
proposal::unsignedPayload(env, pay(target, bob, XRP(1)), selfTicketSeq);
env(proposal::create(target, tx, expiration),
ticket::Use(selfTicketSeq),
Ter(temMALFORMED));
env.close();
BEAST_EXPECT(!proposal::entry(env, target, selfTicketSeq));
BEAST_EXPECT(env.le(keylet::ticket(target.id(), selfTicketSeq)));
BEAST_EXPECT(ownerCount(env, target) == 2);
// Consume the leftover Ticket so target's OwnerCount is back to
// just its original Ticket for the remaining cases below.
env(noop(target), ticket::Use(selfTicketSeq));
env.close();
BEAST_EXPECT(ownerCount(env, target) == 1);
}
// A payload that fails its own transaction type's preflight surfaces
// that type's own code, not a generic error (On-Chain Cosigner spec §5.3.1.2).
{
@@ -195,6 +243,17 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite
// 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.
//
// The rejection code is not uniform, though. A fill that leaves actual
// signature bytes behind (a non-empty TxnSignature, or one inside a
// Signers entry) is, for some containers, intercepted before ever
// reaching our own hasSignatureField check: the payload's own top-level
// fields are checked by Transactor::preflight2's dry-run simulate-key
// logic, and LoanSet forwards its CounterpartySignature through that same
// logic, both yielding temINVALID instead. SponsorSignature and
// BatchSigners are not inspected that way — only their presence is
// checked elsewhere — so they always reach our own check regardless of
// what they hold.
void
testRejectedSignatureFields(FeatureBitset features)
{
@@ -250,8 +309,8 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite
proposal::innerTx(pay(target, bob, drops(++paid)), env.seq(target) + 1)});
};
auto reject = [&](json::Value const& proposedTx) {
env(proposal::create(target, proposedTx, expiration), Ter(temBAD_SIGNER));
auto reject = [&](json::Value const& proposedTx, TER expected) {
env(proposal::create(target, proposedTx, expiration), Ter(expected));
env.close();
BEAST_EXPECT(!proposal::entry(env, target, targetTicketSeq));
BEAST_EXPECT(ownerCount(env, target) == 1);
@@ -259,38 +318,55 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite
// 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<std::function<void(json::Value&)>> 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;
},
// same fills apply to both. `signs` marks a fill that leaves actual
// signature bytes behind, which some containers' own dry-run
// simulate-key check reacts to (see the class comment above).
struct Fill
{
std::function<void(json::Value&)> apply;
bool signs;
};
std::vector<Fill> const fills{
{.apply = [&](json::Value& o) { o[jss::SigningPubKey] = key; }, .signs = false},
{.apply = [&](json::Value& o) { o[sfTxnSignature.getJsonName()] = sig; },
.signs = true},
{.apply =
[&](json::Value& o) {
o[jss::SigningPubKey] = "";
o[sfTxnSignature.getJsonName()] = sig;
},
.signs = true},
// 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;
},
{.apply =
[&](json::Value& o) {
o[jss::SigningPubKey] = key;
o[sfTxnSignature.getJsonName()] = sig;
},
.signs = true},
// 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;
},
{.apply =
[&](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;
},
.signs = true},
{.apply =
[&](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;
},
.signs = true},
};
// Every place a signature could sit, on a payload of a type that
@@ -298,27 +374,35 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite
// 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).
// (On-Chain Cosigner spec §6.1, §6.6.3). `checksSignatureContent`
// marks a place whose own preflight forwards the container through a
// dry-run simulate-key check, the same as the payload's own top-level
// fields.
struct Place
{
std::function<json::Value()> payload;
std::function<json::Value&(json::Value&)> at;
bool checksSignatureContent;
};
std::vector<Place> const places{
{.payload = payment, .at = [](json::Value& tx) -> json::Value& { return tx; }},
{.payload = payment,
.at = [](json::Value& tx) -> json::Value& { return tx; },
.checksSignatureContent = true},
{.payload = loanSet,
.at = [](json::Value& tx) -> json::Value& {
auto& o = tx[sfCounterpartySignature.getJsonName()];
o = json::Value{json::ValueType::Object};
return o;
}},
},
.checksSignatureContent = true},
{.payload = sponsoredPayment,
.at = [](json::Value& tx) -> json::Value& {
auto& o = tx[sfSponsorSignature.getJsonName()];
o = json::Value{json::ValueType::Object};
return o;
}},
},
.checksSignatureContent = false},
// 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.
@@ -327,7 +411,8 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite
auto& o = tx[sfBatchSigners.getJsonName()][0u][sfBatchSigner.getJsonName()];
o[jss::Account] = bob.human();
return o;
}},
},
.checksSignatureContent = false},
};
for (auto const& place : places)
@@ -335,36 +420,40 @@ struct TransactionProposalCreate_test : public beast::unit_test::Suite
for (auto const& fill : fills)
{
json::Value tx = place.payload();
fill(place.at(tx));
reject(tx);
fill.apply(place.at(tx));
reject(tx, place.checksSignatureContent && fill.signs ? temINVALID : temBAD_SIGNER);
}
}
// 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.
// the canonical form. An empty container never trips a simulate-key
// check, so this is temBAD_SIGNER regardless of checksSignatureContent.
for (std::size_t i = 1; i < places.size(); ++i)
{
json::Value tx = places[i].payload();
places[i].at(tx);
reject(tx);
reject(tx, temBAD_SIGNER);
}
// 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.
// without a signature beside it. SigningPubKey is a required common
// field, so its absence means proposedTx isn't a valid instance of its
// own type; that's caught while constructing it as an STTx, before
// reaching our own hasEmptySigningPubKey check.
{
json::Value tx = payment();
tx.removeMember(jss::SigningPubKey);
reject(tx);
reject(tx, temMALFORMED);
}
{
json::Value tx = payment();
tx.removeMember(jss::SigningPubKey);
tx[sfTxnSignature.getJsonName()] = sig;
reject(tx);
reject(tx, temMALFORMED);
}
}