mirror of
https://github.com/XRPLF/rippled.git
synced 2026-02-27 09:12:33 +00:00
feature BatchV1_1
This commit is contained in:
@@ -15,10 +15,9 @@
|
||||
|
||||
// Add new amendments to the top of this list.
|
||||
// Keep it sorted in reverse chronological order.
|
||||
|
||||
XRPL_FEATURE(BatchV1_1, Supported::no, VoteBehavior::DefaultNo)
|
||||
XRPL_FIX (PermissionedDomainInvariant, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FIX (ExpiredNFTokenOfferRemoval, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FIX (BatchInnerSigs, Supported::no, VoteBehavior::DefaultNo)
|
||||
XRPL_FEATURE(LendingProtocol, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FEATURE(PermissionDelegationV1_1, Supported::no, VoteBehavior::DefaultNo)
|
||||
XRPL_FIX (DirectoryLimit, Supported::yes, VoteBehavior::DefaultNo)
|
||||
@@ -32,7 +31,6 @@ XRPL_FEATURE(TokenEscrow, Supported::yes, VoteBehavior::DefaultNo
|
||||
XRPL_FIX (EnforceNFTokenTrustlineV2, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FIX (AMMv1_3, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FEATURE(PermissionedDEX, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FEATURE(Batch, Supported::no, VoteBehavior::DefaultNo)
|
||||
XRPL_FEATURE(SingleAssetVault, Supported::yes, VoteBehavior::DefaultNo)
|
||||
XRPL_FIX (PayChanCancelAfter, Supported::yes, VoteBehavior::DefaultNo)
|
||||
// Check flags in Credential transactions
|
||||
|
||||
@@ -918,7 +918,7 @@ TRANSACTION(ttVAULT_CLAWBACK, 70, VaultClawback,
|
||||
#endif
|
||||
TRANSACTION(ttBATCH, 71, Batch,
|
||||
Delegation::notDelegable,
|
||||
featureBatch,
|
||||
featureBatchV1_1,
|
||||
noPriv,
|
||||
({
|
||||
{sfRawTransactions, soeREQUIRED},
|
||||
|
||||
@@ -175,12 +175,11 @@ Transactor::preflight1(PreflightContext const& ctx, std::uint32_t flagMask)
|
||||
if (ctx.tx.getSeqProxy().isTicket() && ctx.tx.isFieldPresent(sfAccountTxnID))
|
||||
return temINVALID;
|
||||
|
||||
if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatch))
|
||||
if (ctx.tx.isFlag(tfInnerBatchTxn) && !ctx.rules.enabled(featureBatchV1_1))
|
||||
return temINVALID_FLAG;
|
||||
|
||||
XRPL_ASSERT(
|
||||
ctx.tx.isFlag(tfInnerBatchTxn) == ctx.parentBatchId.has_value() ||
|
||||
!ctx.rules.enabled(featureBatch),
|
||||
ctx.tx.isFlag(tfInnerBatchTxn) == ctx.parentBatchId.has_value() || !ctx.rules.enabled(featureBatchV1_1),
|
||||
"Inner batch transaction must have a parent batch ID.");
|
||||
|
||||
return tesSUCCESS;
|
||||
@@ -196,13 +195,13 @@ Transactor::preflight2(PreflightContext const& ctx)
|
||||
return *ret;
|
||||
|
||||
// It should be impossible for the InnerBatchTxn flag to be set without
|
||||
// featureBatch being enabled
|
||||
// featureBatchV1_1 being enabled
|
||||
XRPL_ASSERT_PARTS(
|
||||
!ctx.tx.isFlag(tfInnerBatchTxn) || ctx.rules.enabled(featureBatch),
|
||||
!ctx.tx.isFlag(tfInnerBatchTxn) || ctx.rules.enabled(featureBatchV1_1),
|
||||
"xrpl::Transactor::preflight2",
|
||||
"InnerBatch flag only set if feature enabled");
|
||||
// Skip signature check on batch inner transactions
|
||||
if (ctx.tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatch))
|
||||
if (ctx.tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatchV1_1))
|
||||
return tesSUCCESS;
|
||||
// Do not add any checks after this point that are relevant for
|
||||
// batch inner transactions. They will be skipped.
|
||||
@@ -647,7 +646,7 @@ Transactor::checkSign(
|
||||
|
||||
auto const pkSigner = sigObject.getFieldVL(sfSigningPubKey);
|
||||
// Ignore signature check on batch inner transactions
|
||||
if (parentBatchId && view.rules().enabled(featureBatch))
|
||||
if (parentBatchId && view.rules().enabled(featureBatchV1_1))
|
||||
{
|
||||
// Defensive Check: These values are also checked in Batch::preflight
|
||||
if (sigObject.isFieldPresent(sfTxnSignature) || !pkSigner.empty() ||
|
||||
@@ -731,13 +730,15 @@ Transactor::checkBatchSign(PreclaimContext const& ctx)
|
||||
{
|
||||
if (idAccount != idSigner)
|
||||
return tefBAD_AUTH;
|
||||
|
||||
return tesSUCCESS;
|
||||
}
|
||||
else
|
||||
{
|
||||
if (isPseudoAccount(sleAccount))
|
||||
return tefBAD_AUTH;
|
||||
|
||||
if (ret = checkSingleSign(ctx.view, idSigner, idAccount, sleAccount, ctx.j);
|
||||
!isTesSuccess(ret))
|
||||
return ret;
|
||||
if (ret = checkSingleSign(ctx.view, idSigner, idAccount, sleAccount, ctx.j); !isTesSuccess(ret))
|
||||
return ret;
|
||||
}
|
||||
}
|
||||
}
|
||||
return ret;
|
||||
|
||||
@@ -24,29 +24,12 @@ checkValidity(HashRouter& router, STTx const& tx, Rules const& rules)
|
||||
auto const flags = router.getFlags(id);
|
||||
|
||||
// Ignore signature check on batch inner transactions
|
||||
if (tx.isFlag(tfInnerBatchTxn) && rules.enabled(featureBatch))
|
||||
if (tx.isFlag(tfInnerBatchTxn) && rules.enabled(featureBatchV1_1))
|
||||
{
|
||||
// Defensive Check: These values are also checked in Batch::preflight
|
||||
if (tx.isFieldPresent(sfTxnSignature) || !tx.getSigningPubKey().empty() ||
|
||||
tx.isFieldPresent(sfSigners))
|
||||
return {Validity::SigBad, "Malformed: Invalid inner batch transaction."};
|
||||
|
||||
// This block should probably have never been included in the
|
||||
// original `Batch` implementation. An inner transaction never
|
||||
// has a valid signature.
|
||||
bool const neverValid = rules.enabled(fixBatchInnerSigs);
|
||||
if (!neverValid)
|
||||
{
|
||||
std::string reason;
|
||||
if (!passesLocalChecks(tx, reason))
|
||||
{
|
||||
router.setFlags(id, SF_LOCALBAD);
|
||||
return {Validity::SigGoodOnly, reason};
|
||||
}
|
||||
|
||||
router.setFlags(id, SF_SIGGOOD);
|
||||
return {Validity::Valid, ""};
|
||||
}
|
||||
}
|
||||
|
||||
if (any(flags & SF_SIGBAD))
|
||||
|
||||
@@ -26,8 +26,7 @@ LoanSet::preflight(PreflightContext const& ctx)
|
||||
auto const& tx = ctx.tx;
|
||||
|
||||
// Special case for Batch inner transactions
|
||||
if (tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatch) &&
|
||||
!tx.isFieldPresent(sfCounterparty))
|
||||
if (tx.isFlag(tfInnerBatchTxn) && ctx.rules.enabled(featureBatchV1_1) && !tx.isFieldPresent(sfCounterparty))
|
||||
{
|
||||
auto const parentBatchId = ctx.parentBatchId.value_or(uint256{0});
|
||||
JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]: "
|
||||
|
||||
@@ -141,14 +141,11 @@ class Batch_test : public beast::unit_test::suite
|
||||
using namespace test::jtx;
|
||||
using namespace std::literals;
|
||||
|
||||
bool const withInnerSigFix = features[fixBatchInnerSigs];
|
||||
|
||||
for (bool const withBatch : {true, false})
|
||||
{
|
||||
testcase << "enabled: Batch " << (withBatch ? "enabled" : "disabled")
|
||||
<< ", Inner Sig Fix: " << (withInnerSigFix ? "enabled" : "disabled");
|
||||
testcase << "enabled: Batch " << (withBatch ? "enabled" : "disabled");
|
||||
|
||||
auto const amend = withBatch ? features : features - featureBatch;
|
||||
auto const amend = withBatch ? features : features - featureBatchV1_1;
|
||||
|
||||
test::jtx::Env env{*this, amend};
|
||||
|
||||
@@ -2191,22 +2188,16 @@ class Batch_test : public beast::unit_test::suite
|
||||
void
|
||||
doTestInnerSubmitRPC(FeatureBitset features, bool withBatch)
|
||||
{
|
||||
bool const withInnerSigFix = features[fixBatchInnerSigs];
|
||||
std::string const testName =
|
||||
std::string("inner submit rpc: batch ") + (withBatch ? "enabled" : "disabled") + ": ";
|
||||
|
||||
std::string const testName = [&]() {
|
||||
std::stringstream ss;
|
||||
ss << "inner submit rpc: batch " << (withBatch ? "enabled" : "disabled")
|
||||
<< ", inner sig fix: " << (withInnerSigFix ? "enabled" : "disabled") << ": ";
|
||||
return ss.str();
|
||||
}();
|
||||
|
||||
auto const amend = withBatch ? features : features - featureBatch;
|
||||
auto const amend = withBatch ? features : features - featureBatchV1_1;
|
||||
|
||||
using namespace test::jtx;
|
||||
using namespace std::literals;
|
||||
|
||||
test::jtx::Env env{*this, amend};
|
||||
if (!BEAST_EXPECT(amend[featureBatch] == withBatch))
|
||||
if (!BEAST_EXPECT(amend[featureBatchV1_1] == withBatch))
|
||||
return;
|
||||
|
||||
auto const alice = Account("alice");
|
||||
@@ -2328,8 +2319,7 @@ class Batch_test : public beast::unit_test::suite
|
||||
s.slice(),
|
||||
__LINE__,
|
||||
"fails local checks: Empty SigningPubKey.",
|
||||
"fails local checks: Empty SigningPubKey.",
|
||||
withBatch && !withInnerSigFix);
|
||||
"fails local checks: Empty SigningPubKey.");
|
||||
}
|
||||
|
||||
// Invalid RPC Submission: tfInnerBatchTxn pseudo-transaction
|
||||
@@ -2340,7 +2330,7 @@ class Batch_test : public beast::unit_test::suite
|
||||
{
|
||||
STTx amendTx(ttAMENDMENT, [seq = env.closed()->header().seq + 1](auto& obj) {
|
||||
obj.setAccountID(sfAccount, AccountID());
|
||||
obj.setFieldH256(sfAmendment, fixBatchInnerSigs);
|
||||
obj.setFieldH256(sfAmendment, featureBatchV1_1);
|
||||
obj.setFieldU32(sfLedgerSequence, seq);
|
||||
obj.setFieldU32(sfFlags, tfInnerBatchTxn);
|
||||
});
|
||||
@@ -2352,8 +2342,7 @@ class Batch_test : public beast::unit_test::suite
|
||||
"Pseudo-transaction",
|
||||
s.slice(),
|
||||
__LINE__,
|
||||
withInnerSigFix ? "fails local checks: Empty SigningPubKey."
|
||||
: "fails local checks: Cannot submit pseudo transactions.",
|
||||
"fails local checks: Empty SigningPubKey.",
|
||||
"fails local checks: Empty SigningPubKey.");
|
||||
}
|
||||
}
|
||||
@@ -2414,6 +2403,53 @@ class Batch_test : public beast::unit_test::suite
|
||||
BEAST_EXPECT(env.balance(bob) == XRP(1000));
|
||||
}
|
||||
|
||||
void
|
||||
testCheckAllSignatures(FeatureBitset features)
|
||||
{
|
||||
testcase("check all signatures");
|
||||
|
||||
using namespace test::jtx;
|
||||
using namespace std::literals;
|
||||
|
||||
// Verifies that checkBatchSign validates all signers even when an
|
||||
// unfunded account (signed with its master key) appears first in the
|
||||
// sorted signer list. A funded account with an invalid signature must
|
||||
// still be rejected with tefBAD_AUTH.
|
||||
|
||||
test::jtx::Env env{*this, features};
|
||||
|
||||
auto const alice = Account("alice");
|
||||
// "aaa" sorts before other accounts alphabetically, ensuring the
|
||||
// unfunded account is checked first in the sorted signer list
|
||||
auto const unfunded = Account("aaa");
|
||||
auto const carol = Account("carol");
|
||||
env.fund(XRP(10000), alice, carol);
|
||||
env.close();
|
||||
|
||||
// Verify sort order: unfunded.id() < carol.id()
|
||||
BEAST_EXPECT(unfunded.id() < carol.id());
|
||||
|
||||
auto const seq = env.seq(alice);
|
||||
auto const ledSeq = env.current()->seq();
|
||||
auto const batchFee = batch::calcBatchFee(env, 2, 3);
|
||||
|
||||
// The batch includes:
|
||||
// 1. alice pays unfunded (to create unfunded's account)
|
||||
// 2. unfunded does a noop (signed by unfunded's master key - valid)
|
||||
// 3. carol pays alice (signed by alice's key - INVALID since alice is
|
||||
// not carol's regular key)
|
||||
//
|
||||
// checkBatchSign must validate all signers regardless of order.
|
||||
// This must fail with tefBAD_AUTH.
|
||||
env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
|
||||
batch::inner(pay(alice, unfunded, XRP(100)), seq + 1),
|
||||
batch::inner(noop(unfunded), ledSeq),
|
||||
batch::inner(pay(carol, alice, XRP(1000)), env.seq(carol)),
|
||||
batch::sig(unfunded, Reg{carol, alice}),
|
||||
ter(tefBAD_AUTH));
|
||||
env.close();
|
||||
}
|
||||
|
||||
void
|
||||
testAccountSet(FeatureBitset features)
|
||||
{
|
||||
@@ -4331,6 +4367,7 @@ class Batch_test : public beast::unit_test::suite
|
||||
testIndependent(features);
|
||||
testInnerSubmitRPC(features);
|
||||
testAccountActivation(features);
|
||||
testCheckAllSignatures(features);
|
||||
testAccountSet(features);
|
||||
testAccountDelete(features);
|
||||
testLoan(features);
|
||||
@@ -4356,7 +4393,6 @@ public:
|
||||
{
|
||||
using namespace test::jtx;
|
||||
auto const sa = testable_amendments();
|
||||
testWithFeats(sa - fixBatchInnerSigs);
|
||||
testWithFeats(sa);
|
||||
}
|
||||
};
|
||||
|
||||
@@ -114,9 +114,8 @@ class Feature_test : public beast::unit_test::suite
|
||||
|
||||
// Test a random sampling of the variables. If any of these get retired
|
||||
// or removed, swap out for any other feature.
|
||||
BEAST_EXPECT(
|
||||
featureToName(fixRemoveNFTokenAutoTrustLine) == "fixRemoveNFTokenAutoTrustLine");
|
||||
BEAST_EXPECT(featureToName(featureBatch) == "Batch");
|
||||
BEAST_EXPECT(featureToName(fixRemoveNFTokenAutoTrustLine) == "fixRemoveNFTokenAutoTrustLine");
|
||||
BEAST_EXPECT(featureToName(featureBatchV1_1) == "BatchV1_1");
|
||||
BEAST_EXPECT(featureToName(featureDID) == "DID");
|
||||
BEAST_EXPECT(featureToName(fixIncludeKeyletFields) == "fixIncludeKeyletFields");
|
||||
BEAST_EXPECT(featureToName(featureTokenEscrow) == "TokenEscrow");
|
||||
|
||||
@@ -1119,7 +1119,7 @@ NetworkOPsImp::submitTransaction(std::shared_ptr<STTx const> const& iTrans)
|
||||
}
|
||||
|
||||
// Enforce Network bar for batch txn
|
||||
if (iTrans->isFlag(tfInnerBatchTxn) && m_ledgerMaster.getValidatedRules().enabled(featureBatch))
|
||||
if (iTrans->isFlag(tfInnerBatchTxn) && m_ledgerMaster.getValidatedRules().enabled(featureBatchV1_1))
|
||||
{
|
||||
JLOG(m_journal.error()) << "Submitted transaction invalid: tfInnerBatchTxn flag present.";
|
||||
return;
|
||||
@@ -1185,7 +1185,7 @@ NetworkOPsImp::preProcessTransaction(std::shared_ptr<Transaction>& transaction)
|
||||
// under no circumstances will we ever accept an inner txn within a batch
|
||||
// txn from the network.
|
||||
auto const sttx = *transaction->getSTransaction();
|
||||
if (sttx.isFlag(tfInnerBatchTxn) && view->rules().enabled(featureBatch))
|
||||
if (sttx.isFlag(tfInnerBatchTxn) && view->rules().enabled(featureBatchV1_1))
|
||||
{
|
||||
transaction->setStatus(INVALID);
|
||||
transaction->setResult(temINVALID_FLAG);
|
||||
|
||||
@@ -1291,7 +1291,7 @@ PeerImp::handleTransaction(
|
||||
// Charge strongly for attempting to relay a txn with tfInnerBatchTxn
|
||||
// LCOV_EXCL_START
|
||||
/*
|
||||
There is no need to check whether the featureBatch amendment is
|
||||
There is no need to check whether the featureBatchV1_1 amendment is
|
||||
enabled.
|
||||
|
||||
* If the `tfInnerBatchTxn` flag is set, and the amendment is
|
||||
@@ -2740,7 +2740,7 @@ PeerImp::checkTransaction(
|
||||
// charge strongly for relaying batch txns
|
||||
// LCOV_EXCL_START
|
||||
/*
|
||||
There is no need to check whether the featureBatch amendment is
|
||||
There is no need to check whether the featureBatchV1_1 amendment is
|
||||
enabled.
|
||||
|
||||
* If the `tfInnerBatchTxn` flag is set, and the amendment is
|
||||
|
||||
Reference in New Issue
Block a user