ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2026-07-27 17:10:46 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Mark SAV and Lending transactions as NotDelegable (#6489)

Browse Source 
New transactions should be marked as `NotDelegable`, until the interactions with other transactions have been fully tested and validated.
This commit is contained in:
yinyiqian1
2026-03-11 17:27:35 -04:00
committed by GitHub
parent ce9ccf844a
commit 46d5c67a8d
3 changed files with 67 additions and 148 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -830,7 +830,7 @@ TRANSACTION(ttDELEGATE_SET, 64, DelegateSet,
# include <xrpl/tx/transactors/vault/VaultCreate.h>
#endif
TRANSACTION(ttVAULT_CREATE, 65, VaultCreate,
Delegation::delegable,
Delegation::notDelegable,
featureSingleAssetVault,
createPseudoAcct | createMPTIssuance | mustModifyVault,
({
@@ -848,7 +848,7 @@ TRANSACTION(ttVAULT_CREATE, 65, VaultCreate,
# include <xrpl/tx/transactors/vault/VaultSet.h>
#endif
TRANSACTION(ttVAULT_SET, 66, VaultSet,
Delegation::delegable,
Delegation::notDelegable,
featureSingleAssetVault,
mustModifyVault,
({
@@ -863,7 +863,7 @@ TRANSACTION(ttVAULT_SET, 66, VaultSet,
# include <xrpl/tx/transactors/vault/VaultDelete.h>
#endif
TRANSACTION(ttVAULT_DELETE, 67, VaultDelete,
Delegation::delegable,
Delegation::notDelegable,
featureSingleAssetVault,
mustDeleteAcct | destroyMPTIssuance | mustModifyVault,
({
@@ -875,7 +875,7 @@ TRANSACTION(ttVAULT_DELETE, 67, VaultDelete,
# include <xrpl/tx/transactors/vault/VaultDeposit.h>
#endif
TRANSACTION(ttVAULT_DEPOSIT, 68, VaultDeposit,
Delegation::delegable,
Delegation::notDelegable,
featureSingleAssetVault,
mayAuthorizeMPT | mustModifyVault,
({
@@ -888,7 +888,7 @@ TRANSACTION(ttVAULT_DEPOSIT, 68, VaultDeposit,
# include <xrpl/tx/transactors/vault/VaultWithdraw.h>
#endif
TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw,
Delegation::delegable,
Delegation::notDelegable,
featureSingleAssetVault,
mayDeleteMPT | mayAuthorizeMPT | mustModifyVault,
({
@@ -903,7 +903,7 @@ TRANSACTION(ttVAULT_WITHDRAW, 69, VaultWithdraw,
# include <xrpl/tx/transactors/vault/VaultClawback.h>
#endif
TRANSACTION(ttVAULT_CLAWBACK, 70, VaultClawback,
Delegation::delegable,
Delegation::notDelegable,
featureSingleAssetVault,
mayDeleteMPT | mustModifyVault,
({
@@ -932,7 +932,7 @@ TRANSACTION(ttBATCH, 71, Batch,
# include <xrpl/tx/transactors/lending/LoanBrokerSet.h>
#endif
TRANSACTION(ttLOAN_BROKER_SET, 74, LoanBrokerSet,
Delegation::delegable,
Delegation::notDelegable,
featureLendingProtocol,
createPseudoAcct | mayAuthorizeMPT, ({
{sfVaultID, soeREQUIRED},
@@ -949,7 +949,7 @@ TRANSACTION(ttLOAN_BROKER_SET, 74, LoanBrokerSet,
# include <xrpl/tx/transactors/lending/LoanBrokerDelete.h>
#endif
TRANSACTION(ttLOAN_BROKER_DELETE, 75, LoanBrokerDelete,
Delegation::delegable,
Delegation::notDelegable,
featureLendingProtocol,
mustDeleteAcct | mayAuthorizeMPT, ({
{sfLoanBrokerID, soeREQUIRED},
@@ -960,7 +960,7 @@ TRANSACTION(ttLOAN_BROKER_DELETE, 75, LoanBrokerDelete,
# include <xrpl/tx/transactors/lending/LoanBrokerCoverDeposit.h>
#endif
TRANSACTION(ttLOAN_BROKER_COVER_DEPOSIT, 76, LoanBrokerCoverDeposit,
Delegation::delegable,
Delegation::notDelegable,
featureLendingProtocol,
noPriv, ({
{sfLoanBrokerID, soeREQUIRED},
@@ -972,7 +972,7 @@ TRANSACTION(ttLOAN_BROKER_COVER_DEPOSIT, 76, LoanBrokerCoverDeposit,
# include <xrpl/tx/transactors/lending/LoanBrokerCoverWithdraw.h>
#endif
TRANSACTION(ttLOAN_BROKER_COVER_WITHDRAW, 77, LoanBrokerCoverWithdraw,
Delegation::delegable,
Delegation::notDelegable,
featureLendingProtocol,
mayAuthorizeMPT, ({
{sfLoanBrokerID, soeREQUIRED},
@@ -987,7 +987,7 @@ TRANSACTION(ttLOAN_BROKER_COVER_WITHDRAW, 77, LoanBrokerCoverWithdraw,
# include <xrpl/tx/transactors/lending/LoanBrokerCoverClawback.h>
#endif
TRANSACTION(ttLOAN_BROKER_COVER_CLAWBACK, 78, LoanBrokerCoverClawback,
Delegation::delegable,
Delegation::notDelegable,
featureLendingProtocol,
noPriv, ({
{sfLoanBrokerID, soeOPTIONAL},
@@ -999,7 +999,7 @@ TRANSACTION(ttLOAN_BROKER_COVER_CLAWBACK, 78, LoanBrokerCoverClawback,
# include <xrpl/tx/transactors/lending/LoanSet.h>
#endif
TRANSACTION(ttLOAN_SET, 80, LoanSet,
Delegation::delegable,
Delegation::notDelegable,
featureLendingProtocol,
mayAuthorizeMPT | mustModifyVault, ({
{sfLoanBrokerID, soeREQUIRED},
@@ -1026,7 +1026,7 @@ TRANSACTION(ttLOAN_SET, 80, LoanSet,
# include <xrpl/tx/transactors/lending/LoanDelete.h>
#endif
TRANSACTION(ttLOAN_DELETE, 81, LoanDelete,
Delegation::delegable,
Delegation::notDelegable,
featureLendingProtocol,
noPriv, ({
{sfLoanID, soeREQUIRED},
@@ -1037,7 +1037,7 @@ TRANSACTION(ttLOAN_DELETE, 81, LoanDelete,
# include <xrpl/tx/transactors/lending/LoanManage.h>
#endif
TRANSACTION(ttLOAN_MANAGE, 82, LoanManage,
Delegation::delegable,
Delegation::notDelegable,
featureLendingProtocol,
// All of the LoanManage options will modify the vault, but the
// transaction can succeed without options, essentially making it
@@ -1051,7 +1051,7 @@ TRANSACTION(ttLOAN_MANAGE, 82, LoanManage,
# include <xrpl/tx/transactors/lending/LoanPay.h>
#endif
TRANSACTION(ttLOAN_PAY, 84, LoanPay,
Delegation::delegable,
Delegation::notDelegable,
featureLendingProtocol,
mayAuthorizeMPT | mustModifyVault, ({
{sfLoanID, soeREQUIRED},

59
src/test/app/Delegate_test.cpp
View File

@@ -1614,13 +1614,7 @@ class Delegate_test : public beast::unit_test::suite
{"CredentialDelete", featureCredentials},
{"NFTokenModify", featureDynamicNFT},
{"PermissionedDomainSet", featurePermissionedDomains},
{"PermissionedDomainDelete", featurePermissionedDomains},
{"VaultCreate", featureSingleAssetVault},
{"VaultSet", featureSingleAssetVault},
{"VaultDelete", featureSingleAssetVault},
{"VaultDeposit", featureSingleAssetVault},
{"VaultWithdraw", featureSingleAssetVault},
{"VaultClawback", featureSingleAssetVault}};
{"PermissionedDomainDelete", featurePermissionedDomains}};
// Can not delegate tx if any required feature disabled.
{
@@ -1660,6 +1654,56 @@ class Delegate_test : public beast::unit_test::suite
}
}
void
testTxDelegableCount()
{
testcase("Delegable Transactions Completeness");
std::size_t delegableCount = 0;
#pragma push_macro("TRANSACTION")
#undef TRANSACTION
#define TRANSACTION(tag, value, name, delegable, ...) \
if (delegable == xrpl::delegable) \
{ \
delegableCount++; \
}
#include <xrpl/protocol/detail/transactions.macro>
#undef TRANSACTION
#pragma pop_macro("TRANSACTION")
// ====================================================================
// IMPORTANT NOTICE:
//
// If this test fails, it indicates that the 'Delegation::delegable' status
// in transactions.macro has been changed. Delegation allows accounts to act
// on behalf of others, significantly increasing the security surface.
//
//
// To ENSURE any added transaction is safe and compatible with delegation:
//
// 1. Verify that the transaction is intended to be delegable.
// 2. Every standard test case for that transaction MUST be
// duplicated and verified for a Delegated context.
// 3. Ensure that Fee, Reserve, and Signing are correctly handled.
//
// DO NOT modify expectedDelegableCount unless all scenarios, including
// edge cases, have been fully tested and verified.
// ====================================================================
std::size_t const expectedDelegableCount = 75;
BEAST_EXPECTS(
delegableCount == expectedDelegableCount,
"\n[SECURITY] New delegable transaction detected!"
"\n Expected: " +
std::to_string(expectedDelegableCount) +
"\n Actual: " + std::to_string(delegableCount) +
"\n Action: Verify security requirements to interact with Delegation feature");
}
void
run() override
{
@@ -1684,6 +1728,7 @@ class Delegate_test : public beast::unit_test::suite
testMultiSignQuorumNotMet();
testPermissionValue(all);
testTxRequireFeatures(all);
testTxDelegableCount();
}
};
BEAST_DEFINE_TESTSUITE(Delegate, app, xrpl);

126
src/test/app/Vault_test.cpp
View File

@@ -4500,131 +4500,6 @@ class Vault_test : public beast::unit_test::suite
}
}
void
testDelegate()
{
using namespace test::jtx;
Env env(*this, testable_amendments());
Account alice{"alice"};
Account bob{"bob"};
Account carol{"carol"};
struct CaseArgs
{
PrettyAsset asset = xrpIssue();
};
auto const xrpBalance = [this](
Env const& env, Account const& account) -> std::optional<long> {
auto sle = env.le(keylet::account(account.id()));
if (BEAST_EXPECT(sle != nullptr))
return sle->getFieldAmount(sfBalance).xrp().drops();
return std::nullopt;
};
auto testCase = [&, this](auto test, CaseArgs args = {}) {
Env env{*this, testable_amendments() | featureSingleAssetVault};
Vault vault{env};
// use different initial amount to distinguish the source balance
env.fund(XRP(10000), alice);
env.fund(XRP(20000), bob);
env.fund(XRP(30000), carol);
env.close();
env(delegate::set(
carol,
alice,
{"Payment",
"VaultCreate",
"VaultSet",
"VaultDelete",
"VaultDeposit",
"VaultWithdraw",
"VaultClawback"}));
test(env, vault, args.asset);
};
testCase([&, this](Env& env, Vault& vault, PrettyAsset const& asset) {
testcase("delegated vault creation");
auto startBalance = xrpBalance(env, carol);
if (!BEAST_EXPECT(startBalance.has_value()))
return;
auto [tx, keylet] = vault.create({.owner = carol, .asset = asset});
env(tx, delegate::as(alice));
env.close();
BEAST_EXPECT(xrpBalance(env, carol) == *startBalance);
});
testCase([&, this](Env& env, Vault& vault, PrettyAsset const& asset) {
testcase("delegated deposit and withdrawal");
auto [tx, keylet] = vault.create({.owner = carol, .asset = asset});
env(tx);
env.close();
auto const amount = 1513;
auto const baseFee = env.current()->fees().base;
auto startBalance = xrpBalance(env, carol);
if (!BEAST_EXPECT(startBalance.has_value()))
return;
tx = vault.deposit({.depositor = carol, .id = keylet.key, .amount = asset(amount)});
env(tx, delegate::as(alice));
env.close();
BEAST_EXPECT(xrpBalance(env, carol) == *startBalance - amount);
tx =
vault.withdraw({.depositor = carol, .id = keylet.key, .amount = asset(amount - 1)});
env(tx, delegate::as(alice));
env.close();
BEAST_EXPECT(xrpBalance(env, carol) == *startBalance - 1);
tx = vault.withdraw({.depositor = carol, .id = keylet.key, .amount = asset(1)});
env(tx);
env.close();
BEAST_EXPECT(xrpBalance(env, carol) == *startBalance - baseFee);
});
testCase([&, this](Env& env, Vault& vault, PrettyAsset const& asset) {
testcase("delegated withdrawal same as base fee and deletion");
auto [tx, keylet] = vault.create({.owner = carol, .asset = asset});
env(tx);
env.close();
auto const amount = 25537;
auto const baseFee = env.current()->fees().base;
auto startBalance = xrpBalance(env, carol);
if (!BEAST_EXPECT(startBalance.has_value()))
return;
tx = vault.deposit({.depositor = carol, .id = keylet.key, .amount = asset(amount)});
env(tx);
env.close();
BEAST_EXPECT(xrpBalance(env, carol) == *startBalance - amount - baseFee);
tx = vault.withdraw({.depositor = carol, .id = keylet.key, .amount = asset(baseFee)});
env(tx, delegate::as(alice));
env.close();
BEAST_EXPECT(xrpBalance(env, carol) == *startBalance - amount);
tx = vault.withdraw(
{.depositor = carol, .id = keylet.key, .amount = asset(amount - baseFee)});
env(tx, delegate::as(alice));
env.close();
BEAST_EXPECT(xrpBalance(env, carol) == *startBalance - baseFee);
tx = vault.del({.owner = carol, .id = keylet.key});
env(tx, delegate::as(alice));
env.close();
});
}
void
testVaultClawbackBurnShares()
{
@@ -5374,7 +5249,6 @@ public:
testFailedPseudoAccount();
testScaleIOU();
testRPC();
testDelegate();
testVaultClawbackBurnShares();
testVaultClawbackAssets();
testAssetsMaximum();