ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-06-02 16:26:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix clang & address review comments

Browse Source
This commit is contained in:
Denis Angell
2026-05-20 12:32:49 +02:00
parent 33b406ee87
commit c8efacc8dc
10 changed files with 116 additions and 66 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
include/xrpl/protocol/detail/transactions.macro
View File

@@ -936,7 +936,7 @@ TRANSACTION(ttVAULT_CLAWBACK, 70, VaultClawback,
#endif
TRANSACTION(ttBATCH, 71, Batch,
Delegation::NotDelegable,
featureBatch,
featureBatchV1_1,
NoPriv,
({
{sfRawTransactions, SoeRequired},

2
include/xrpl/protocol_autogen/transactions/Batch.h
View File

@@ -20,7 +20,7 @@ class BatchBuilder;
*
* Type: ttBATCH (71)
* Delegable: Delegation::NotDelegable
* Amendment: featureBatch
* Amendment: featureBatchV1_1
* Privileges: NoPriv
*
* Immutable wrapper around STTx providing type-safe field access.

2
src/libxrpl/protocol/STTx.cpp
View File

@@ -570,7 +570,7 @@ STTx::buildBatchTxnIds()
auto const& raw = getFieldArray(sfRawTransactions);
if (raw.size() > maxBatchTxCount)
if (raw.size() > kMaxBatchTxCount)
return;
batchTxnIds_.reserve(raw.size());

12
src/libxrpl/tx/transactors/payment/Payment.cpp
View File

@@ -330,7 +330,11 @@ Payment::preclaim(PreclaimContext const& ctx)
// transaction would succeed.
return tecNO_DST;
}
if (partialPaymentAllowed)
// Prior to featureBatchV1_1 this check was skipped on closed views,
// which let inner batch payments slip through. Gate the broadened
// check on the amendment so behavior is preserved pre-amendment.
if (partialPaymentAllowed &&
(ctx.view.open() || ctx.view.rules().enabled(featureBatchV1_1)))
{
// You cannot fund an account with a partial payment.
// Make retry work smaller, by rejecting this.
@@ -367,7 +371,11 @@ Payment::preclaim(PreclaimContext const& ctx)
}
// Payment with at least one intermediate step and uses transitive balances.
if (hasPaths || sendMax || !dstAmount.native())
// Prior to featureBatchV1_1 the path size check below was only enforced on
// open views; gate the broadened enforcement on the amendment so
// pre-amendment behavior is preserved.
if ((hasPaths || sendMax || !dstAmount.native()) &&
(ctx.view.open() || ctx.view.rules().enabled(featureBatchV1_1)))
{
STPathSet const& paths = ctx.tx.getFieldPathSet(sfPaths);

8
src/libxrpl/tx/transactors/system/Batch.cpp
View File

@@ -136,11 +136,11 @@ Batch::calculateBaseFee(ReadView const& view, STTx const& tx)
{
auto const& nestedSigners = signer.getFieldArray(sfSigners);
// LCOV_EXCL_START
if (nestedSigners.size() > STTx::maxMultiSigners)
if (nestedSigners.size() > STTx::kMaxMultiSigners)
{
JLOG(debugLog().error())
<< "BatchTrace: Nested Signers array exceeds max entries.";
return XRPAmount{INITIAL_XRP};
return kInitialXrp;
}
// LCOV_EXCL_STOP
signerCount += nestedSigners.size();
@@ -243,7 +243,7 @@ Batch::preflight(PreflightContext const& ctx)
}
if (ctx.tx.isFieldPresent(sfBatchSigners) &&
ctx.tx.getFieldArray(sfBatchSigners).size() > maxBatchTxCount)
ctx.tx.getFieldArray(sfBatchSigners).size() > kMaxBatchTxCount)
{
JLOG(ctx.j.debug()) << "BatchTrace[" << parentBatchId << "]:"
<< "signers array exceeds 8 entries.";
@@ -442,7 +442,7 @@ Batch::preflightSigValidated(PreflightContext const& ctx)
}
// Batch signers must be in strictly ascending order by AccountID.
AccountID lastBatchSigner(beast::zero);
AccountID lastBatchSigner{beast::kZero};
for (auto const& signer : signers)
{
AccountID const signerAccount = signer.getAccountID(sfAccount);

94
src/test/app/Batch_test.cpp
View File

@@ -51,6 +51,7 @@
#include <xrpl/protocol/STTx.h>
#include <xrpl/protocol/SecretKey.h>
#include <xrpl/protocol/Serializer.h>
#include <xrpl/protocol/Sign.h>
#include <xrpl/protocol/SystemParameters.h>
#include <xrpl/protocol/TER.h>
#include <xrpl/protocol/TxFlags.h>
@@ -429,9 +430,9 @@ class Batch_test : public beast::unit_test::Suite
// temINVALID_INNER_BATCH: tfInnerBatchTxn set but no parentBatchId.
{
auto jtx = env.jt(pay(alice, bob, XRP(1)), txflags(tfInnerBatchTxn));
auto jtx = env.jt(pay(alice, bob, XRP(1)), Txflags(tfInnerBatchTxn));
PreflightContext pfCtx(
env.app(), *jtx.stx, env.current()->rules(), tapNONE, env.journal);
env.app(), *jtx.stx, env.current()->rules(), TapNone, env.journal);
auto const pf = Transactor::invokePreflight<Payment>(pfCtx);
BEAST_EXPECT(pf == temINVALID_INNER_BATCH);
}
@@ -441,7 +442,7 @@ class Batch_test : public beast::unit_test::Suite
{
auto jtx = env.jt(pay(alice, bob, XRP(1)));
PreflightContext pfCtx(
env.app(), *jtx.stx, uint256{1}, env.current()->rules(), tapBATCH, env.journal);
env.app(), *jtx.stx, uint256{1}, env.current()->rules(), TapBatch, env.journal);
auto const pf = Transactor::invokePreflight<Payment>(pfCtx);
BEAST_EXPECT(pf == temINVALID_INNER_BATCH);
}
@@ -1002,16 +1003,16 @@ class Batch_test : public beast::unit_test::Suite
auto const batchFee = batch::calcBatchFee(env, 0, 2);
auto const seq = env.seq(alice);
Json::Value tx1;
json::Value tx1;
tx1[jss::TransactionType] = jss::OfferCreate;
tx1[jss::Account] = alice.human();
tx1[jss::TakerPays] = mptAmt.getJson(JsonOptions::none);
tx1[jss::TakerGets] = XRP(10).value().getJson(JsonOptions::none);
tx1[jss::TakerPays] = mptAmt.getJson(JsonOptions::Values::None);
tx1[jss::TakerGets] = XRP(10).value().getJson(JsonOptions::Values::None);
env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::inner(tx1, seq + 1),
batch::inner(pay(alice, bob, XRP(1)), seq + 2),
ter(telENV_RPC_FAILED));
batch::Inner(tx1, seq + 1),
batch::Inner(pay(alice, bob, XRP(1)), seq + 2),
Ter(telENV_RPC_FAILED));
env.close();
}
@@ -1028,9 +1029,9 @@ class Batch_test : public beast::unit_test::Suite
m["MemoData"] = strHex(std::string("test"));
env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::inner(tx1, seq + 1),
batch::inner(pay(alice, bob, XRP(1)), seq + 2),
ter(telENV_RPC_FAILED));
batch::Inner(tx1, seq + 1),
batch::Inner(pay(alice, bob, XRP(1)), seq + 2),
Ter(telENV_RPC_FAILED));
env.close();
}
}
@@ -2566,11 +2567,11 @@ class Batch_test : public beast::unit_test::Suite
// 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));
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();
}
@@ -4497,7 +4498,7 @@ class Batch_test : public beast::unit_test::Suite
// Submit a normal Payment with tfInnerBatchTxn flag.
// preflight1 must reject with temINVALID_INNER_BATCH because
// the flag is set but no parentBatchId exists.
env(pay(alice, bob, XRP(1)), txflags(tfInnerBatchTxn), ter(telENV_RPC_FAILED));
env(pay(alice, bob, XRP(1)), Txflags(tfInnerBatchTxn), Ter(telENV_RPC_FAILED));
env.close();
// Verify via direct apply path (bypassing RPC layer)
@@ -4514,7 +4515,7 @@ class Batch_test : public beast::unit_test::Suite
obj.setFieldU32(sfFlags, tfInnerBatchTxn);
});
auto const result = xrpl::apply(env.app(), view, stx, tapNONE, j);
auto const result = xrpl::apply(env.app(), view, stx, TapNone, j);
// Must NOT be applied — signature was never checked
BEAST_EXPECT(!result.applied);
return false;
@@ -4549,13 +4550,13 @@ class Batch_test : public beast::unit_test::Suite
auto const batchFee1 = batch::calcBatchFee(env, 2, 2);
auto jt1 = env.jt(
batch::outer(alice, aliceSeq, batchFee1, tfOnlyOne),
batch::inner(pay(bob, alice, XRP(100)), bobSeq),
batch::inner(pay(carol, alice, XRP(50)), carolSeq),
batch::sig(bob, carol));
batch::Inner(pay(bob, alice, XRP(100)), bobSeq),
batch::Inner(pay(carol, alice, XRP(50)), carolSeq),
batch::Sig(bob, carol));
auto const capturedSigners = jt1.jv[sfBatchSigners.jsonName];
env(jt1, ter(tesSUCCESS));
env(jt1, Ter(tesSUCCESS));
env.close();
BEAST_EXPECT(env.seq(bob) == bobSeq + 1);
BEAST_EXPECT(env.seq(carol) == carolSeq);
@@ -4563,11 +4564,11 @@ class Batch_test : public beast::unit_test::Suite
auto const batchFee2 = batch::calcBatchFee(env, 2, 2);
auto jt2 = env.jtnofill(
batch::outer(eve, env.seq(eve), batchFee2, tfOnlyOne),
batch::inner(pay(bob, alice, XRP(100)), bobSeq),
batch::inner(pay(carol, alice, XRP(50)), carolSeq));
batch::Inner(pay(bob, alice, XRP(100)), bobSeq),
batch::Inner(pay(carol, alice, XRP(50)), carolSeq));
jt2.jv[sfBatchSigners.jsonName] = capturedSigners;
env(jt2.jv, ter(temBAD_SIGNATURE));
env(jt2.jv, Ter(temBAD_SIGNATURE));
env.close();
BEAST_EXPECT(env.seq(carol) == carolSeq);
@@ -4593,13 +4594,13 @@ class Batch_test : public beast::unit_test::Suite
auto const batchFee1 = batch::calcBatchFee(env, 2, 2);
auto jt1 = env.jt(
batch::outer(alice, aliceSeq1, batchFee1, tfOnlyOne),
batch::inner(pay(bob, alice, XRP(500)), bobSeq),
batch::inner(pay(carol, alice, XRP(500)), carolSeq),
batch::sig(bob, carol));
batch::Inner(pay(bob, alice, XRP(500)), bobSeq),
batch::Inner(pay(carol, alice, XRP(500)), carolSeq),
batch::Sig(bob, carol));
auto const capturedSigners = jt1.jv[sfBatchSigners.jsonName];
env(jt1, ter(tesSUCCESS));
env(jt1, Ter(tesSUCCESS));
env.close();
BEAST_EXPECT(env.seq(bob) == bobSeq + 1);
BEAST_EXPECT(env.seq(carol) == carolSeq);
@@ -4607,11 +4608,11 @@ class Batch_test : public beast::unit_test::Suite
auto const batchFee2 = batch::calcBatchFee(env, 2, 2);
auto jt2 = env.jtnofill(
batch::outer(alice, env.seq(alice), batchFee2, tfOnlyOne),
batch::inner(pay(bob, alice, XRP(500)), bobSeq),
batch::inner(pay(carol, alice, XRP(500)), carolSeq));
batch::Inner(pay(bob, alice, XRP(500)), bobSeq),
batch::Inner(pay(carol, alice, XRP(500)), carolSeq));
jt2.jv[sfBatchSigners.jsonName] = capturedSigners;
env(jt2.jv, ter(temBAD_SIGNATURE));
env(jt2.jv, Ter(temBAD_SIGNATURE));
env.close();
BEAST_EXPECT(env.balance(carol) == preCarol);
@@ -4640,31 +4641,31 @@ class Batch_test : public beast::unit_test::Suite
auto const batchFee = batch::calcBatchFee(env, 3, 2);
auto jt1 = env.jt(
batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::inner(pay(alice, bob, XRP(10)), seq + 1),
batch::inner(pay(bob, alice, XRP(5)), env.seq(bob)),
batch::msig(bob, {dave, elsa}),
ter(tesSUCCESS));
batch::Inner(pay(alice, bob, XRP(10)), seq + 1),
batch::Inner(pay(bob, alice, XRP(5)), env.seq(bob)),
batch::Msig(bob, {dave, elsa}),
Ter(tesSUCCESS));
auto const bobSignerEntry = jt1.jv[sfBatchSigners.jsonName][0u];
env(jt1, ter(tesSUCCESS));
env(jt1, Ter(tesSUCCESS));
env.close();
auto const seq2 = env.seq(alice);
auto const batchFee2 = batch::calcBatchFee(env, 3, 2);
auto jt2 = env.jtnofill(
batch::outer(alice, seq2, batchFee2, tfAllOrNothing),
batch::inner(pay(alice, carol, XRP(10)), seq2 + 1),
batch::inner(pay(carol, alice, XRP(5)), env.seq(carol)));
batch::Inner(pay(alice, carol, XRP(10)), seq2 + 1),
batch::Inner(pay(carol, alice, XRP(5)), env.seq(carol)));
Json::Value carolSigner;
json::Value carolSigner;
carolSigner[sfBatchSigner.jsonName][jss::Account] = carol.human();
carolSigner[sfBatchSigner.jsonName][jss::SigningPubKey] = "";
carolSigner[sfBatchSigner.jsonName][sfSigners.jsonName] =
bobSignerEntry[sfBatchSigner.jsonName][sfSigners.jsonName];
jt2.jv[sfBatchSigners.jsonName][0u] = carolSigner;
env(jt2.jv, ter(temBAD_SIGNATURE));
env(jt2.jv, Ter(temBAD_SIGNATURE));
env.close();
}
}
@@ -4691,16 +4692,16 @@ class Batch_test : public beast::unit_test::Suite
auto jt = env.jt(
batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::inner(pay(bob, alice, XRP(10)), bobSeq),
batch::inner(pay(carol, alice, XRP(5)), carolSeq),
batch::sig(bob, carol));
batch::Inner(pay(bob, alice, XRP(10)), bobSeq),
batch::Inner(pay(carol, alice, XRP(5)), carolSeq),
batch::Sig(bob, carol));
auto const s0 = jt.jv[sfBatchSigners.jsonName][0u];
auto const s1 = jt.jv[sfBatchSigners.jsonName][1u];
jt.jv[sfBatchSigners.jsonName][0u] = s1;
jt.jv[sfBatchSigners.jsonName][1u] = s0;
env(jt.jv, ter(temBAD_SIGNER));
env(jt.jv, Ter(temBAD_SIGNER));
env.close();
}
@@ -4750,7 +4751,6 @@ public:
using namespace test::jtx;
auto const sa = testableAmendments();
testWithFeats(sa - featureBatchV1_1);
testWithFeats(sa);
}
};

14
src/test/app/LedgerReplay_test.cpp
View File

@@ -1,12 +1,15 @@
#include <test/jtx/Account.h>
#include <test/jtx/Env.h>
#include <test/jtx/TestHelpers.h>
#include <test/jtx/amount.h>
#include <test/jtx/batch.h>
#include <test/jtx/envconfig.h>
#include <test/jtx/fee.h>
#include <test/jtx/pay.h>
#include <test/jtx/seq.h>
#include <test/jtx/sig.h>
#include <test/jtx/tags.h>
#include <test/jtx/ter.h>
#include <xrpld/app/ledger/BuildLedger.h>
#include <xrpld/app/ledger/InboundLedger.h>
@@ -99,7 +102,7 @@ struct LedgerReplay_test : public beast::unit_test::Suite
using namespace jtx;
Env env(*this, testable_amendments());
Env env(*this, testableAmendments());
auto const alice = Account("alice");
auto const bob = Account("bob");
@@ -109,10 +112,9 @@ struct LedgerReplay_test : public beast::unit_test::Suite
auto const seq = env.seq(alice);
auto const batchFee = batch::calcBatchFee(env, 0, 2);
env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
batch::inner(pay(alice, bob, XRP(1)), seq + 1),
batch::inner(pay(alice, bob, XRP(2)), seq + 2),
batch::sig(alice),
ter(tesSUCCESS));
batch::Inner(pay(alice, bob, XRP(1)), seq + 1),
batch::Inner(pay(alice, bob, XRP(2)), seq + 2),
Ter(tesSUCCESS));
env.close();
LedgerMaster& ledgerMaster = env.app().getLedgerMaster();
@@ -120,7 +122,7 @@ struct LedgerReplay_test : public beast::unit_test::Suite
auto const lastClosedParent = ledgerMaster.getLedgerByHash(lastClosed->header().parentHash);
auto const replayed = buildLedger(
LedgerReplay(lastClosedParent, lastClosed), tapNONE, env.app(), env.journal);
LedgerReplay(lastClosedParent, lastClosed), TapNone, env.app(), env.journal);
BEAST_EXPECT(replayed->header().hash == lastClosed->header().hash);
}

39
src/test/ledger/View_test.cpp
View File

@@ -1046,6 +1046,44 @@ class View_test : public beast::unit_test::Suite
}
}
// Verifies that OpenView::txExists() walks its parent chain so a tx that
// was applied in a previous ledger is still reported as existing from a
// fresh OpenView built over that ledger. This is what lets
// Transactor::checkSeq return tefALREADY for a replay against the closed
// ledger.
void
testTxExistsRecurses()
{
testcase("OpenView::txExists walks parent chain");
using namespace jtx;
Env env(*this);
auto const alice = Account("alice");
env.fund(XRP(10000), alice);
env.close();
// Build the noop tx so we know its txid up-front.
auto const jt = env.jt(noop(alice));
auto const txid = jt.stx->getTransactionID();
// Before submission, no view knows about this tx.
BEAST_EXPECT(!env.current()->txExists(txid));
BEAST_EXPECT(!env.closed()->txExists(txid));
// Submit and close so the tx ends up in the closed ledger but no
// longer in the open view's local txs_.
env(jt.jv);
env.close();
BEAST_EXPECT(env.closed()->txExists(txid));
// A fresh OpenView built over the closed ledger has an empty txs_
// map, but must still report the tx as existing via its base.
auto const ov = env.current();
BEAST_EXPECT(ov->txExists(txid));
}
void
run() override
{
@@ -1063,6 +1101,7 @@ class View_test : public beast::unit_test::Suite
testTransferRate();
testAreCompatible();
testRegressions();
testTxExistsRecurses();
}
};

4
src/test/rpc/Simulate_test.cpp
View File

@@ -421,8 +421,8 @@ class Simulate_test : public beast::unit_test::Suite
}
{
// tfInnerBatchTxn flag on top-level transaction
Json::Value params;
Json::Value tx_json = Json::objectValue;
json::Value params;
json::Value tx_json{json::ValueType::Object};
tx_json[jss::TransactionType] = jss::AccountSet;
tx_json[jss::Account] = env.master.human();
tx_json[jss::Flags] = tfInnerBatchTxn;

5
src/xrpld/rpc/handlers/transaction/Simulate.cpp
View File

@@ -359,8 +359,9 @@ doSimulate(RPC::JsonContext& context)
// Reject transactions with the tfInnerBatchTxn flag.
if (stTx->isFlag(tfInnerBatchTxn))
{
return RPC::make_error(
rpcINVALID_PARAMS, "tfInnerBatchTxn flag is not allowed on top-level transactions.");
return RPC::makeError(
RpcInvalidParams,
"tfInnerBatchTxn flag is not allowed on top-level transactions.");
}
std::string reason;