ARCHIVE/rippled
1
0
Fork 0
mirror of https://github.com/XRPLF/rippled.git synced 2025-12-06 17:27:55 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: Add restrictions to Permission Delegation: fixDelegateV1_1 (#5650)

Browse Source 
- Amendment: fixDelegateV1_1
- In DelegateSet, disallow invalid PermissionValues like 0, and transaction values when the transaction's amendment is not enabled. Acts as if the transaction doesn't exist, which is the same thing older versions without the amendment will do.
- Payment burn/mint should disallow DEX currency exchange.
- Support MPT for Payment burn/mint.
This commit is contained in:
yinyiqian1
2025-09-10 13:47:33 -04:00
committed by GitHub
parent 3d92375d12
commit 61d628d654
12 changed files with 732 additions and 102 deletions
Download Patch File Download Diff File Expand all files Collapse all files

42
src/libxrpl/protocol/Permissions.cpp
View File

@@ -18,6 +18,7 @@
//==============================================================================
#include <xrpl/beast/utility/instrumentation.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Permissions.h>
#include <xrpl/protocol/jss.h>
@@ -25,11 +26,24 @@ namespace ripple {
Permission::Permission()
{
txFeatureMap_ = {
#pragma push_macro("TRANSACTION")
#undef TRANSACTION
#define TRANSACTION(tag, value, name, delegatable, amendment, ...) \
{value, amendment},
#include <xrpl/protocol/detail/transactions.macro>
#undef TRANSACTION
#pragma pop_macro("TRANSACTION")
};
delegatableTx_ = {
#pragma push_macro("TRANSACTION")
#undef TRANSACTION
#define TRANSACTION(tag, value, name, delegatable, fields) {value, delegatable},
#define TRANSACTION(tag, value, name, delegatable, ...) {value, delegatable},
#include <xrpl/protocol/detail/transactions.macro>
@@ -118,7 +132,9 @@ Permission::getGranularTxType(GranularPermissionType const& gpType) const
}
bool
Permission::isDelegatable(std::uint32_t const& permissionValue) const
Permission::isDelegatable(
std::uint32_t const& permissionValue,
Rules const& rules) const
{
auto const granularPermission =
getGranularName(static_cast<GranularPermissionType>(permissionValue));
@@ -126,7 +142,27 @@ Permission::isDelegatable(std::uint32_t const& permissionValue) const
// granular permissions are always allowed to be delegated
return true;
auto const it = delegatableTx_.find(permissionValue - 1);
auto const txType = permissionToTxType(permissionValue);
auto const it = delegatableTx_.find(txType);
if (rules.enabled(fixDelegateV1_1))
{
if (it == delegatableTx_.end())
return false;
auto const txFeaturesIt = txFeatureMap_.find(txType);
XRPL_ASSERT(
txFeaturesIt != txFeatureMap_.end(),
"ripple::Permissions::isDelegatable : tx exists in txFeatureMap_");
// fixDelegateV1_1: Delegation is only allowed if the required amendment
// for the transaction is enabled. For transactions that do not require
// an amendment, delegation is always allowed.
if (txFeaturesIt->second != uint256{} &&
!rules.enabled(txFeaturesIt->second))
return false;
}
if (it != delegatableTx_.end() && it->second == Delegation::notDelegatable)
return false;

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

@@ -55,7 +55,7 @@ TxFormats::TxFormats()
#undef TRANSACTION
#define UNWRAP(...) __VA_ARGS__
#define TRANSACTION(tag, value, name, delegatable, fields) \
#define TRANSACTION(tag, value, name, delegatable, amendment, fields) \
add(jss::name, tag, UNWRAP fields, commonFields);
#include <xrpl/protocol/detail/transactions.macro>

3
src/test/app/AMMClawback_test.cpp
View File

@@ -2442,8 +2442,7 @@ class AMMClawback_test : public beast::unit_test::suite
void
run() override
{
FeatureBitset const all{
jtx::testable_amendments() | fixAMMClawbackRounding};
FeatureBitset const all = jtx::testable_amendments();
testInvalidRequest();
testFeatureDisabled(all - featureAMMClawback);

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

@@ -16,6 +16,7 @@
//==============================================================================
#include <test/jtx.h>
#include <test/jtx/CaptureLogs.h>
#include <test/jtx/delegate.h>
#include <xrpl/protocol/Feature.h>
@@ -139,12 +140,12 @@ class Delegate_test : public beast::unit_test::suite
}
void
testInvalidRequest()
testInvalidRequest(FeatureBitset features)
{
testcase("test invalid DelegateSet");
using namespace jtx;
Env env(*this);
Env env(*this, features);
Account gw{"gateway"};
Account alice{"alice"};
Account bob{"bob"};
@@ -216,22 +217,17 @@ class Delegate_test : public beast::unit_test::suite
}
// non-delegatable transaction
auto const res = features[fixDelegateV1_1] ? ter(temMALFORMED)
: ter(tecNO_PERMISSION);
{
env(delegate::set(gw, alice, {"SetRegularKey"}),
ter(tecNO_PERMISSION));
env(delegate::set(gw, alice, {"AccountSet"}),
ter(tecNO_PERMISSION));
env(delegate::set(gw, alice, {"SignerListSet"}),
ter(tecNO_PERMISSION));
env(delegate::set(gw, alice, {"DelegateSet"}),
ter(tecNO_PERMISSION));
env(delegate::set(gw, alice, {"SetRegularKey"}),
ter(tecNO_PERMISSION));
env(delegate::set(gw, alice, {"EnableAmendment"}),
ter(tecNO_PERMISSION));
env(delegate::set(gw, alice, {"UNLModify"}), ter(tecNO_PERMISSION));
env(delegate::set(gw, alice, {"SetFee"}), ter(tecNO_PERMISSION));
env(delegate::set(gw, alice, {"Batch"}), ter(tecNO_PERMISSION));
env(delegate::set(gw, alice, {"SetRegularKey"}), res);
env(delegate::set(gw, alice, {"AccountSet"}), res);
env(delegate::set(gw, alice, {"SignerListSet"}), res);
env(delegate::set(gw, alice, {"DelegateSet"}), res);
env(delegate::set(gw, alice, {"EnableAmendment"}), res);
env(delegate::set(gw, alice, {"UNLModify"}), res);
env(delegate::set(gw, alice, {"SetFee"}), res);
env(delegate::set(gw, alice, {"Batch"}), res);
}
}
@@ -536,7 +532,7 @@ class Delegate_test : public beast::unit_test::suite
}
void
testPaymentGranular()
testPaymentGranular(FeatureBitset features)
{
testcase("test payment granular");
using namespace jtx;
@@ -706,6 +702,158 @@ class Delegate_test : public beast::unit_test::suite
env.require(balance(alice, USD(50)));
BEAST_EXPECT(env.balance(bob, USD) == USD(0));
}
// disallow cross currency payment with only PaymentBurn/PaymentMint
// permission
{
Env env(*this, features);
Account const alice{"alice"};
Account const bob{"bob"};
Account const gw{"gateway"};
Account const carol{"carol"};
auto const USD = gw["USD"];
env.fund(XRP(10000), alice, bob, carol, gw);
env.close();
env.trust(USD(50000), alice);
env.trust(USD(50000), bob);
env.trust(USD(50000), carol);
env(pay(gw, alice, USD(10000)));
env(pay(gw, bob, USD(10000)));
env(pay(gw, carol, USD(10000)));
env.close();
auto const result = features[fixDelegateV1_1]
? static_cast<TER>(tecNO_DELEGATE_PERMISSION)
: static_cast<TER>(tesSUCCESS);
auto const offerCount = features[fixDelegateV1_1] ? 1 : 0;
// PaymentMint
{
env(offer(carol, XRP(100), USD(501)));
BEAST_EXPECT(expectOffers(env, carol, 1));
env(delegate::set(gw, bob, {"PaymentMint"}));
env.close();
// post-amendment: fixDelegateV1_1
// bob can not send cross currency payment on behalf of the gw,
// even with PaymentMint permission and gw being the issuer.
env(pay(gw, alice, USD(5000)),
path(~USD),
sendmax(XRP(1001)),
txflags(tfPartialPayment),
delegate::as(bob),
ter(result));
BEAST_EXPECT(expectOffers(env, carol, offerCount));
// succeed with direct payment
env(pay(gw, alice, USD(100)), delegate::as(bob));
env.close();
}
// PaymentBurn
{
env(offer(bob, XRP(100), USD(501)));
BEAST_EXPECT(expectOffers(env, bob, 1));
env(delegate::set(alice, bob, {"PaymentBurn"}));
env.close();
// post-amendment: fixDelegateV1_1
// bob can not send cross currency payment on behalf of alice,
// even with PaymentBurn permission and gw being the issuer.
env(pay(alice, gw, USD(5000)),
path(~USD),
sendmax(XRP(1001)),
txflags(tfPartialPayment),
delegate::as(bob),
ter(result));
BEAST_EXPECT(expectOffers(env, bob, offerCount));
// succeed with direct payment
env(pay(alice, gw, USD(100)), delegate::as(bob));
env.close();
}
}
// PaymentMint and PaymentBurn for MPT
{
std::string logs;
Env env(*this, features, std::make_unique<CaptureLogs>(&logs));
Account const alice{"alice"};
Account const bob{"bob"};
Account const gw{"gateway"};
MPTTester mpt(env, gw, {.holders = {alice, bob}});
mpt.create(
{.ownerCount = 1, .holderCount = 0, .flags = tfMPTCanTransfer});
mpt.authorize({.account = alice});
mpt.authorize({.account = bob});
auto const MPT = mpt["MPT"];
env(pay(gw, alice, MPT(500)));
env(pay(gw, bob, MPT(500)));
env.close();
auto aliceMPT = env.balance(alice, MPT);
auto bobMPT = env.balance(bob, MPT);
// PaymentMint
{
env(delegate::set(gw, bob, {"PaymentMint"}));
env.close();
if (!features[fixDelegateV1_1])
{
// pre-amendment: PaymentMint is not supported for MPT
env(pay(gw, alice, MPT(50)),
delegate::as(bob),
ter(tefEXCEPTION));
}
else
{
env(pay(gw, alice, MPT(50)), delegate::as(bob));
BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT + MPT(50));
BEAST_EXPECT(env.balance(bob, MPT) == bobMPT);
aliceMPT = env.balance(alice, MPT);
}
}
// PaymentBurn
{
env(delegate::set(alice, bob, {"PaymentBurn"}));
env.close();
if (!features[fixDelegateV1_1])
{
// pre-amendment: PaymentBurn is not supported for MPT
env(pay(alice, gw, MPT(50)),
delegate::as(bob),
ter(tefEXCEPTION));
}
else
{
env(pay(alice, gw, MPT(50)), delegate::as(bob));
BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT - MPT(50));
BEAST_EXPECT(env.balance(bob, MPT) == bobMPT);
aliceMPT = env.balance(alice, MPT);
}
}
// Payment transaction for MPT is allowed for both pre and post
// amendment
{
env(delegate::set(
alice, bob, {"PaymentBurn", "PaymentMint", "Payment"}));
env.close();
env(pay(alice, gw, MPT(50)), delegate::as(bob));
BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT - MPT(50));
BEAST_EXPECT(env.balance(bob, MPT) == bobMPT);
aliceMPT = env.balance(alice, MPT);
env(pay(alice, bob, MPT(100)), delegate::as(bob));
BEAST_EXPECT(env.balance(alice, MPT) == aliceMPT - MPT(100));
BEAST_EXPECT(env.balance(bob, MPT) == bobMPT + MPT(100));
}
}
}
void
@@ -1476,18 +1624,216 @@ class Delegate_test : public beast::unit_test::suite
BEAST_EXPECT(env.balance(edward) == edwardBalance);
}
void
testPermissionValue(FeatureBitset features)
{
testcase("test permission value");
using namespace jtx;
Env env(*this, features);
Account alice{"alice"};
Account bob{"bob"};
env.fund(XRP(100000), alice, bob);
env.close();
auto buildRequest = [&](auto value) -> Json::Value {
Json::Value jv;
jv[jss::TransactionType] = jss::DelegateSet;
jv[jss::Account] = alice.human();
jv[sfAuthorize.jsonName] = bob.human();
Json::Value permissionsJson(Json::arrayValue);
Json::Value permissionValue;
permissionValue[sfPermissionValue.jsonName] = value;
Json::Value permissionObj;
permissionObj[sfPermission.jsonName] = permissionValue;
permissionsJson.append(permissionObj);
jv[sfPermissions.jsonName] = permissionsJson;
return jv;
};
// invalid permission value.
// neither granular permission nor transaction level permission
for (auto value : {0, 100000, 54321})
{
auto jv = buildRequest(value);
if (!features[fixDelegateV1_1])
env(jv);
else
env(jv, ter(temMALFORMED));
}
}
void
testTxReqireFeatures(FeatureBitset features)
{
testcase("test delegate disabled tx");
using namespace jtx;
// map of tx and required feature.
// non-delegatable tx are not included.
// NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer,
// NFTokenAcceptOffer are not included, they are tested separately.
std::unordered_map<std::string, uint256> txRequiredFeatures{
{"TicketCreate", featureTicketBatch},
{"CheckCreate", featureChecks},
{"CheckCash", featureChecks},
{"CheckCancel", featureChecks},
{"DepositPreauth", featureDepositPreauth},
{"Clawback", featureClawback},
{"AMMClawback", featureAMMClawback},
{"AMMCreate", featureAMM},
{"AMMDeposit", featureAMM},
{"AMMWithdraw", featureAMM},
{"AMMVote", featureAMM},
{"AMMBid", featureAMM},
{"AMMDelete", featureAMM},
{"XChainCreateClaimID", featureXChainBridge},
{"XChainCommit", featureXChainBridge},
{"XChainClaim", featureXChainBridge},
{"XChainAccountCreateCommit", featureXChainBridge},
{"XChainAddClaimAttestation", featureXChainBridge},
{"XChainAddAccountCreateAttestation", featureXChainBridge},
{"XChainModifyBridge", featureXChainBridge},
{"XChainCreateBridge", featureXChainBridge},
{"DIDSet", featureDID},
{"DIDDelete", featureDID},
{"OracleSet", featurePriceOracle},
{"OracleDelete", featurePriceOracle},
{"LedgerStateFix", fixNFTokenPageLinks},
{"MPTokenIssuanceCreate", featureMPTokensV1},
{"MPTokenIssuanceDestroy", featureMPTokensV1},
{"MPTokenIssuanceSet", featureMPTokensV1},
{"MPTokenAuthorize", featureMPTokensV1},
{"CredentialCreate", featureCredentials},
{"CredentialAccept", featureCredentials},
{"CredentialDelete", featureCredentials},
{"NFTokenModify", featureDynamicNFT},
{"PermissionedDomainSet", featurePermissionedDomains},
{"PermissionedDomainDelete", featurePermissionedDomains},
{"VaultCreate", featureSingleAssetVault},
{"VaultSet", featureSingleAssetVault},
{"VaultDelete", featureSingleAssetVault},
{"VaultDeposit", featureSingleAssetVault},
{"VaultWithdraw", featureSingleAssetVault},
{"VaultClawback", featureSingleAssetVault}};
// fixDelegateV1_1 post-amendment: can not delegate tx if any
// required feature disabled.
{
auto txAmendmentDisabled = [&](FeatureBitset features,
std::string const& tx) {
BEAST_EXPECT(txRequiredFeatures.contains(tx));
Env env(*this, features - txRequiredFeatures[tx]);
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(100000), alice, bob);
env.close();
if (!features[fixDelegateV1_1])
env(delegate::set(alice, bob, {tx}));
else
env(delegate::set(alice, bob, {tx}), ter(temMALFORMED));
};
for (auto const& tx : txRequiredFeatures)
txAmendmentDisabled(features, tx.first);
}
// if all the required features in txRequiredFeatures are enabled, will
// succeed
{
auto txAmendmentEnabled = [&](std::string const& tx) {
Env env(*this, features);
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(100000), alice, bob);
env.close();
env(delegate::set(alice, bob, {tx}));
};
for (auto const& tx : txRequiredFeatures)
txAmendmentEnabled(tx.first);
}
// NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, and
// NFTokenAcceptOffer are tested separately. Since
// featureNonFungibleTokensV1_1 includes the functionality of
// featureNonFungibleTokensV1, fixNFTokenNegOffer, and fixNFTokenDirV1,
// both featureNonFungibleTokensV1_1 and featureNonFungibleTokensV1 need
// to be disabled to block these transactions from being delegated.
{
Env env(
*this,
features - featureNonFungibleTokensV1 -
featureNonFungibleTokensV1_1);
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(100000), alice, bob);
env.close();
for (auto const tx :
{"NFTokenMint",
"NFTokenBurn",
"NFTokenCreateOffer",
"NFTokenCancelOffer",
"NFTokenAcceptOffer"})
{
if (!features[fixDelegateV1_1])
env(delegate::set(alice, bob, {tx}));
else
env(delegate::set(alice, bob, {tx}), ter(temMALFORMED));
}
}
// NFTokenMint, NFTokenBurn, NFTokenCreateOffer, NFTokenCancelOffer, and
// NFTokenAcceptOffer are allowed to be delegated if either
// featureNonFungibleTokensV1 or featureNonFungibleTokensV1_1 is
// enabled.
{
for (auto const feature :
{featureNonFungibleTokensV1, featureNonFungibleTokensV1_1})
{
Env env(*this, features - feature);
Account const alice{"alice"};
Account const bob{"bob"};
env.fund(XRP(100000), alice, bob);
env.close();
for (auto const tx :
{"NFTokenMint",
"NFTokenBurn",
"NFTokenCreateOffer",
"NFTokenCancelOffer",
"NFTokenAcceptOffer"})
env(delegate::set(alice, bob, {tx}));
}
}
}
void
run() override
{
FeatureBitset const all = jtx::testable_amendments();
testFeatureDisabled();
testDelegateSet();
testInvalidRequest();
testInvalidRequest(all);
testInvalidRequest(all - fixDelegateV1_1);
testReserve();
testFee();
testSequence();
testAccountDelete();
testDelegateTransaction();
testPaymentGranular();
testPaymentGranular(all);
testPaymentGranular(all - fixDelegateV1_1);
testTrustSetGranular();
testAccountSetGranular();
testMPTokenIssuanceSetGranular();
@@ -1495,6 +1841,10 @@ class Delegate_test : public beast::unit_test::suite
testSingleSignBadSecret();
testMultiSign();
testMultiSignQuorumNotMet();
testPermissionValue(all);
testPermissionValue(all - fixDelegateV1_1);
testTxReqireFeatures(all);
testTxReqireFeatures(all - fixDelegateV1_1);
}
};
BEAST_DEFINE_TESTSUITE(Delegate, app, ripple);

22
src/xrpld/app/tx/detail/DelegateSet.cpp
View File

@@ -23,7 +23,6 @@
#include <xrpl/basics/Log.h>
#include <xrpl/protocol/Feature.h>
#include <xrpl/protocol/Indexes.h>
#include <xrpl/protocol/TxFlags.h>
#include <xrpl/protocol/st.h>
namespace ripple {
@@ -51,6 +50,11 @@ DelegateSet::preflight(PreflightContext const& ctx)
{
if (!permissionSet.insert(permission[sfPermissionValue]).second)
return temMALFORMED;
if (ctx.rules.enabled(fixDelegateV1_1) &&
!Permission::getInstance().isDelegatable(
permission[sfPermissionValue], ctx.rules))
return temMALFORMED;
}
return preflight2(ctx);
@@ -68,9 +72,21 @@ DelegateSet::preclaim(PreclaimContext const& ctx)
auto const& permissions = ctx.tx.getFieldArray(sfPermissions);
for (auto const& permission : permissions)
{
auto const permissionValue = permission[sfPermissionValue];
if (!Permission::getInstance().isDelegatable(permissionValue))
if (!ctx.view.rules().enabled(fixDelegateV1_1) &&
!Permission::getInstance().isDelegatable(
permission[sfPermissionValue], ctx.view.rules()))
{
// Before fixDelegateV1_1:
// - The check was performed during preclaim.
// - Transactions from amendments not yet enabled could still be
// delegated.
//
// After fixDelegateV1_1:
// - The check is performed during preflight.
// - Transactions from amendments not yet enabled can no longer be
// delegated.
return tecNO_PERMISSION;
}
}
return tesSUCCESS;

27
src/xrpld/app/tx/detail/Payment.cpp
View File

@@ -265,8 +265,33 @@ Payment::checkPermission(ReadView const& view, STTx const& tx)
loadGranularPermission(sle, ttPAYMENT, granularPermissions);
auto const& dstAmount = tx.getFieldAmount(sfAmount);
auto const& amountIssue = dstAmount.issue();
// post-amendment: disallow cross currency payments for PaymentMint and
// PaymentBurn
if (view.rules().enabled(fixDelegateV1_1))
{
auto const& amountAsset = dstAmount.asset();
if (tx.isFieldPresent(sfSendMax) &&
tx[sfSendMax].asset() != amountAsset)
return tecNO_DELEGATE_PERMISSION;
if (granularPermissions.contains(PaymentMint) && !isXRP(amountAsset) &&
amountAsset.getIssuer() == tx[sfAccount])
return tesSUCCESS;
if (granularPermissions.contains(PaymentBurn) && !isXRP(amountAsset) &&
amountAsset.getIssuer() == tx[sfDestination])
return tesSUCCESS;
return tecNO_DELEGATE_PERMISSION;
}
// Calling dstAmount.issue() in the next line would throw if it holds MPT.
// That exception would be caught in preclaim and returned as tefEXCEPTION.
// This check is just a cleaner, more explicit way to get the same result.
if (dstAmount.holds<MPTIssue>())
return tefEXCEPTION;
auto const& amountIssue = dstAmount.issue();
if (granularPermissions.contains(PaymentMint) && !isXRP(amountIssue) &&
amountIssue.account == tx[sfAccount])
return tesSUCCESS;

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

@@ -97,8 +97,8 @@ with_txn_type(TxType txnType, F&& f)
#pragma push_macro("TRANSACTION")
#undef TRANSACTION
#define TRANSACTION(tag, value, name, delegatable, fields) \
case tag: \
#define TRANSACTION(tag, value, name, ...) \
case tag: \
return f.template operator()<name>();
#include <xrpl/protocol/detail/transactions.macro>