ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-11-04 11:15:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(amendment): Add missing fields for keylets to ledger objects (#5646)

Browse Source 
This change adds a fix amendment (`fixIncludeKeyletFields`) that adds:
* `sfSequence` to `Escrow` and `PayChannel`
* `sfOwner` to `SignerList`
* `sfOracleDocumentID` to `Oracle`

This ensures that all ledger entries hold all the information needed to determine their keylet.
This commit is contained in:
Mayukha Vadari
2025-09-17 17:34:47 -04:00
committed by GitHub
parent 37b951859c
commit 510314d344
12 changed files with 144 additions and 51 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
include/xrpl/protocol/Indexes.h
View File

@@ -287,9 +287,11 @@ delegate(AccountID const& account, AccountID const& authorizedAccount) noexcept;
Keylet
bridge(STXChainBridge const& bridge, STXChainBridge::ChainType chainType);
// `seq` is stored as `sfXChainClaimID` in the object
Keylet
xChainClaimID(STXChainBridge const& bridge, std::uint64_t seq);
// `seq` is stored as `sfXChainAccountCreateCount` in the object
Keylet
xChainCreateAccountClaimID(STXChainBridge const& bridge, std::uint64_t seq);

1
include/xrpl/protocol/detail/features.macro
View File

@@ -32,6 +32,7 @@
// If you add an amendment here, then do not forget to increment `numFeatures`
// in include/xrpl/protocol/Feature.h.
XRPL_FIX (IncludeKeyletFields, Supported::no, VoteBehavior::DefaultNo)
XRPL_FEATURE(DynamicMPT, Supported::no, VoteBehavior::DefaultNo)
XRPL_FIX (TokenEscrowV1, Supported::yes, VoteBehavior::DefaultNo)
XRPL_FIX (DelegateV1_1, Supported::no, VoteBehavior::DefaultNo)

6
include/xrpl/protocol/detail/ledger_entries.macro
View File

@@ -120,6 +120,7 @@ LEDGER_ENTRY(ltNFTOKEN_PAGE, 0x0050, NFTokenPage, nft_page, ({
// All fields are soeREQUIRED because there is always a SignerEntries.
// If there are no SignerEntries the node is deleted.
LEDGER_ENTRY(ltSIGNER_LIST, 0x0053, SignerList, signer_list, ({
{sfOwner, soeOPTIONAL},
{sfOwnerNode, soeREQUIRED},
{sfSignerQuorum, soeREQUIRED},
{sfSignerEntries, soeREQUIRED},
@@ -188,7 +189,7 @@ LEDGER_ENTRY(ltDIR_NODE, 0x0064, DirectoryNode, directory, ({
{sfNFTokenID, soeOPTIONAL},
{sfPreviousTxnID, soeOPTIONAL},
{sfPreviousTxnLgrSeq, soeOPTIONAL},
{sfDomainID, soeOPTIONAL}
{sfDomainID, soeOPTIONAL} // order book directories
}))
/** The ledger object which lists details about amendments on the network.
@@ -343,6 +344,7 @@ LEDGER_ENTRY(ltXCHAIN_OWNED_CREATE_ACCOUNT_CLAIM_ID, 0x0074, XChainOwnedCreateAc
*/
LEDGER_ENTRY(ltESCROW, 0x0075, Escrow, escrow, ({
{sfAccount, soeREQUIRED},
{sfSequence, soeOPTIONAL},
{sfDestination, soeREQUIRED},
{sfAmount, soeREQUIRED},
{sfCondition, soeOPTIONAL},
@@ -365,6 +367,7 @@ LEDGER_ENTRY(ltESCROW, 0x0075, Escrow, escrow, ({
LEDGER_ENTRY(ltPAYCHAN, 0x0078, PayChannel, payment_channel, ({
{sfAccount, soeREQUIRED},
{sfDestination, soeREQUIRED},
{sfSequence, soeOPTIONAL},
{sfAmount, soeREQUIRED},
{sfBalance, soeREQUIRED},
{sfPublicKey, soeREQUIRED},
@@ -433,6 +436,7 @@ LEDGER_ENTRY(ltMPTOKEN, 0x007f, MPToken, mptoken, ({
*/
LEDGER_ENTRY(ltORACLE, 0x0080, Oracle, oracle, ({
{sfOwner, soeREQUIRED},
{sfOracleDocumentID, soeOPTIONAL},
{sfProvider, soeREQUIRED},
{sfPriceDataSeries, soeREQUIRED},
{sfAssetClass, soeREQUIRED},

9
src/test/app/Escrow_test.cpp
View File

@@ -253,6 +253,14 @@ struct Escrow_test : public beast::unit_test::suite
BEAST_EXPECT(sle);
BEAST_EXPECT((*sle)[sfSourceTag] == 1);
BEAST_EXPECT((*sle)[sfDestinationTag] == 2);
if (features[fixIncludeKeyletFields])
{
BEAST_EXPECT((*sle)[sfSequence] == seq);
}
else
{
BEAST_EXPECT(!sle->isFieldPresent(sfSequence));
}
}
void
@@ -1718,6 +1726,7 @@ public:
FeatureBitset const all{testable_amendments()};
testWithFeats(all);
testWithFeats(all - featureTokenEscrow);
testTags(all - fixIncludeKeyletFields);
}
};

115
src/test/app/MultiSign_test.cpp
View File

@@ -63,7 +63,7 @@ class MultiSign_test : public beast::unit_test::suite
public:
void
test_noReserve(FeatureBitset features)
testNoReserve(FeatureBitset features)
{
testcase("No Reserve");
@@ -133,7 +133,7 @@ public:
}
void
test_signerListSet(FeatureBitset features)
testSignerListSet(FeatureBitset features)
{
testcase("SignerListSet");
@@ -215,7 +215,7 @@ public:
}
void
test_phantomSigners(FeatureBitset features)
testPhantomSigners(FeatureBitset features)
{
testcase("Phantom Signers");
@@ -282,7 +282,7 @@ public:
}
void
test_fee(FeatureBitset features)
testFee(FeatureBitset features)
{
testcase("Fee");
@@ -346,7 +346,7 @@ public:
}
void
test_misorderedSigners(FeatureBitset features)
testMisorderedSigners(FeatureBitset features)
{
testcase("Misordered Signers");
@@ -374,7 +374,7 @@ public:
}
void
test_masterSigners(FeatureBitset features)
testMasterSigners(FeatureBitset features)
{
testcase("Master Signers");
@@ -429,7 +429,7 @@ public:
}
void
test_regularSigners(FeatureBitset features)
testRegularSigners(FeatureBitset features)
{
testcase("Regular Signers");
@@ -494,7 +494,7 @@ public:
}
void
test_regularSignersUsingSubmitMulti(FeatureBitset features)
testRegularSignersUsingSubmitMulti(FeatureBitset features)
{
testcase("Regular Signers Using submit_multisigned");
@@ -734,7 +734,7 @@ public:
}
void
test_heterogeneousSigners(FeatureBitset features)
testHeterogeneousSigners(FeatureBitset features)
{
testcase("Heterogenious Signers");
@@ -881,7 +881,7 @@ public:
// We want to always leave an account signable. Make sure the that we
// disallow removing the last way a transaction may be signed.
void
test_keyDisable(FeatureBitset features)
testKeyDisable(FeatureBitset features)
{
testcase("Key Disable");
@@ -963,7 +963,7 @@ public:
// Verify that the first regular key can be made for free using the
// master key, but not when multisigning.
void
test_regKey(FeatureBitset features)
testRegKey(FeatureBitset features)
{
testcase("Regular Key");
@@ -1000,7 +1000,7 @@ public:
// See if every kind of transaction can be successfully multi-signed.
void
test_txTypes(FeatureBitset features)
testTxTypes(FeatureBitset features)
{
testcase("Transaction Types");
@@ -1089,7 +1089,7 @@ public:
}
void
test_badSignatureText(FeatureBitset features)
testBadSignatureText(FeatureBitset features)
{
testcase("Bad Signature Text");
@@ -1285,7 +1285,7 @@ public:
}
void
test_noMultiSigners(FeatureBitset features)
testNoMultiSigners(FeatureBitset features)
{
testcase("No Multisigners");
@@ -1304,7 +1304,7 @@ public:
}
void
test_multisigningMultisigner(FeatureBitset features)
testMultisigningMultisigner(FeatureBitset features)
{
testcase("Multisigning multisigner");
@@ -1381,7 +1381,7 @@ public:
}
void
test_signForHash(FeatureBitset features)
testSignForHash(FeatureBitset features)
{
testcase("sign_for Hash");
@@ -1464,7 +1464,7 @@ public:
}
void
test_amendmentTransition()
testAmendmentTransition()
{
testcase("Amendment Transition");
@@ -1559,7 +1559,7 @@ public:
}
void
test_signersWithTickets(FeatureBitset features)
testSignersWithTickets(FeatureBitset features)
{
testcase("Signers With Tickets");
@@ -1600,7 +1600,7 @@ public:
}
void
test_signersWithTags(FeatureBitset features)
testSignersWithTags(FeatureBitset features)
{
if (!features[featureExpandedSignerList])
return;
@@ -1680,7 +1680,7 @@ public:
}
void
test_signerListSetFlags(FeatureBitset features)
testSignerListSetFlags(FeatureBitset features)
{
using namespace test::jtx;
@@ -1702,27 +1702,57 @@ public:
env.close();
}
void
testSignerListObject(FeatureBitset features)
{
testcase("SignerList Object");
// Verify that the SignerList object is created correctly.
using namespace jtx;
Env env{*this, features};
Account const alice{"alice", KeyType::ed25519};
env.fund(XRP(1000), alice);
env.close();
// Attach phantom signers to alice.
env(signers(alice, 1, {{bogie, 1}, {demon, 1}}));
env.close();
// Verify that the SignerList object was created correctly.
auto const& sle = env.le(keylet::signers(alice.id()));
BEAST_EXPECT(sle);
BEAST_EXPECT(sle->getFieldArray(sfSignerEntries).size() == 2);
if (features[fixIncludeKeyletFields])
{
BEAST_EXPECT((*sle)[sfOwner] == alice.id());
}
else
{
BEAST_EXPECT(!sle->isFieldPresent(sfOwner));
}
}
void
testAll(FeatureBitset features)
{
test_noReserve(features);
test_signerListSet(features);
test_phantomSigners(features);
test_fee(features);
test_misorderedSigners(features);
test_masterSigners(features);
test_regularSigners(features);
test_regularSignersUsingSubmitMulti(features);
test_heterogeneousSigners(features);
test_keyDisable(features);
test_regKey(features);
test_txTypes(features);
test_badSignatureText(features);
test_noMultiSigners(features);
test_multisigningMultisigner(features);
test_signForHash(features);
test_signersWithTickets(features);
test_signersWithTags(features);
testNoReserve(features);
testSignerListSet(features);
testPhantomSigners(features);
testFee(features);
testMisorderedSigners(features);
testMasterSigners(features);
testRegularSigners(features);
testRegularSignersUsingSubmitMulti(features);
testHeterogeneousSigners(features);
testKeyDisable(features);
testRegKey(features);
testTxTypes(features);
testBadSignatureText(features);
testNoMultiSigners(features);
testMultisigningMultisigner(features);
testSignForHash(features);
testSignersWithTickets(features);
testSignersWithTags(features);
}
void
@@ -1739,10 +1769,13 @@ public:
testAll(all - featureExpandedSignerList);
testAll(all);
test_signerListSetFlags(all - fixInvalidTxFlags);
test_signerListSetFlags(all);
testSignerListSetFlags(all - fixInvalidTxFlags);
testSignerListSetFlags(all);
test_amendmentTransition();
testSignerListObject(all - fixIncludeKeyletFields);
testSignerListObject(all);
testAmendmentTransition();
}
};

23
src/test/app/Oracle_test.cpp
View File

@@ -398,7 +398,7 @@ private:
}
void
testCreate()
testCreate(FeatureBitset features)
{
testcase("Create");
using namespace jtx;
@@ -413,18 +413,30 @@ private:
env, {.owner = owner, .series = series, .fee = baseFee});
BEAST_EXPECT(oracle.exists());
BEAST_EXPECT(ownerCount(env, owner) == (count + adj));
auto const entry = oracle.ledgerEntry();
BEAST_EXPECT(entry[jss::node][jss::Owner] == owner.human());
if (features[fixIncludeKeyletFields])
{
BEAST_EXPECT(
entry[jss::node][jss::OracleDocumentID] ==
oracle.documentID());
}
else
{
BEAST_EXPECT(!entry[jss::node].isMember(jss::OracleDocumentID));
}
BEAST_EXPECT(oracle.expectLastUpdateTime(946694810));
};
{
// owner count is adjusted by 1
Env env(*this);
Env env(*this, features);
test(env, {{"XRP", "USD", 740, 1}}, 1);
}
{
// owner count is adjusted by 2
Env env(*this);
Env env(*this, features);
test(
env,
{{"XRP", "USD", 740, 1},
@@ -438,7 +450,7 @@ private:
{
// Different owner creates a new object
Env env(*this);
Env env(*this, features);
auto const baseFee =
static_cast<int>(env.current()->fees().base.drops());
Account const some("some");
@@ -864,7 +876,8 @@ public:
auto const all = testable_amendments();
testInvalidSet();
testInvalidDelete();
testCreate();
testCreate(all);
testCreate(all - fixIncludeKeyletFields);
testDelete();
testUpdate();
testAmendment();

9
src/test/app/PayChan_test.cpp
View File

@@ -1852,6 +1852,14 @@ struct PayChan_test : public beast::unit_test::suite
BEAST_EXPECT(ownerDirCount(*env.current(), alice) == 1);
BEAST_EXPECT(!inOwnerDir(*env.current(), bob, chanSle));
BEAST_EXPECT(ownerDirCount(*env.current(), bob) == 0);
if (features[fixIncludeKeyletFields])
{
BEAST_EXPECT((*chanSle)[sfSequence] == env.seq(alice) - 1);
}
else
{
BEAST_EXPECT(!chanSle->isFieldPresent(sfSequence));
}
// close the channel
env(claim(bob, chan), txflags(tfClose));
BEAST_EXPECT(!channelExists(*env.current(), chan));
@@ -2348,6 +2356,7 @@ public:
testWithFeats(all - disallowIncoming);
testWithFeats(all);
testDepositAuthCreds();
testMetaAndOwnership(all - fixIncludeKeyletFields);
}
};

4
src/test/jtx/impl/Oracle.cpp
View File

@@ -317,10 +317,10 @@ Oracle::ledgerEntry(
if (jr.isObject())
{
if (jr.isMember(jss::error))
return jr;
if (jr.isMember(jss::result) && jr[jss::result].isMember(jss::status))
return jr[jss::result];
else if (jr.isMember(jss::error))
return jr;
}
return Json::nullValue;
}

5
src/xrpld/app/tx/detail/Escrow.cpp
View File

@@ -538,6 +538,11 @@ EscrowCreate::doApply()
(*slep)[~sfFinishAfter] = ctx_.tx[~sfFinishAfter];
(*slep)[~sfDestinationTag] = ctx_.tx[~sfDestinationTag];
if (ctx_.view().rules().enabled(fixIncludeKeyletFields))
{
(*slep)[sfSequence] = ctx_.tx.getSeqValue();
}
if (ctx_.view().rules().enabled(featureTokenEscrow) && !isXRP(amount))
{
auto const xferRate = transferRate(ctx_.view(), amount);

4
src/xrpld/app/tx/detail/PayChan.cpp
View File

@@ -286,6 +286,10 @@ PayChanCreate::doApply()
(*slep)[~sfCancelAfter] = ctx_.tx[~sfCancelAfter];
(*slep)[~sfSourceTag] = ctx_.tx[~sfSourceTag];
(*slep)[~sfDestinationTag] = ctx_.tx[~sfDestinationTag];
if (ctx_.view().rules().enabled(fixIncludeKeyletFields))
{
(*slep)[sfSequence] = ctx_.tx.getSeqValue();
}
ctx_.view().insert(slep);

9
src/xrpld/app/tx/detail/SetOracle.cpp
View File

@@ -271,6 +271,11 @@ SetOracle::doApply()
if (ctx_.tx.isFieldPresent(sfURI))
sle->setFieldVL(sfURI, ctx_.tx[sfURI]);
sle->setFieldU32(sfLastUpdateTime, ctx_.tx[sfLastUpdateTime]);
if (!sle->isFieldPresent(sfOracleDocumentID) &&
ctx_.view().rules().enabled(fixIncludeKeyletFields))
{
(*sle)[sfOracleDocumentID] = ctx_.tx[sfOracleDocumentID];
}
auto const newCount = pairs.size() > 5 ? 2 : 1;
auto const adjust = newCount - oldCount;
@@ -285,6 +290,10 @@ SetOracle::doApply()
sle = std::make_shared<SLE>(oracleID);
sle->setAccountID(sfOwner, ctx_.tx.getAccountID(sfAccount));
if (ctx_.view().rules().enabled(fixIncludeKeyletFields))
{
(*sle)[sfOracleDocumentID] = ctx_.tx[sfOracleDocumentID];
}
sle->setFieldVL(sfProvider, ctx_.tx[sfProvider]);
if (ctx_.tx.isFieldPresent(sfURI))
sle->setFieldVL(sfURI, ctx_.tx[sfURI]);

8
src/xrpld/app/tx/detail/SetSignerList.cpp
View File

@@ -37,7 +37,7 @@ namespace ripple {
// We're prepared for there to be multiple signer lists in the future,
// but we don't need them yet. So for the time being we're manually
// setting the sfSignerListID to zero in all cases.
static std::uint32_t const defaultSignerListID_ = 0;
static std::uint32_t const DEFAULT_SIGNER_LIST_ID = 0;
std::tuple<
NotTEC,
@@ -424,8 +424,12 @@ SetSignerList::writeSignersToSLE(
std::uint32_t flags) const
{
// Assign the quorum, default SignerListID, and flags.
if (ctx_.view().rules().enabled(fixIncludeKeyletFields))
{
ledgerEntry->setAccountID(sfOwner, account_);
}
ledgerEntry->setFieldU32(sfSignerQuorum, quorum_);
ledgerEntry->setFieldU32(sfSignerListID, defaultSignerListID_);
ledgerEntry->setFieldU32(sfSignerListID, DEFAULT_SIGNER_LIST_ID);
if (flags) // Only set flags if they are non-default (default is zero).
ledgerEntry->setFieldU32(sfFlags, flags);