🚀 Advanced: Support PermissionDelegation Amendment XLS-74d XLS-75d by yinyiqian1 · Pull Request #5354 · XRPLF/rippled · GitHub

yinyiqian1 · 2025-03-19T06:19:33Z

spec:
https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0074d-account-permissions
https://github.com/XRPLF/XRPL-Standards/tree/master/XLS-0075d-permission-delegation

This amendment is called PermissionDelegation

DelegateSet transaction is added so that a delegating account can give permission to delegated account to send transaction on his behalf.

{
    TransactionType: "DelegateSet",
    Account: "rDelegating......",
    Authorize: "rDelegated......",
    Permissions: [{Permission: {PermissionValue: "Payment"}}],
}

Ledger object Delegate is created, its keylet is hashed from delegating and delegated account.
optional common field Delegate is added, to indicate that a transaction is a delegating transaction.
The following transaction is a delegating transaction, Delegate account is the sender of the transaction and will sign this transaction, Account is the delegating account,

{
    Transaction: "Payment",
    Account: "rDelegating......",
    Amount: "1000000000",
    Destination: "r......",
    Delegate: "rDelegated......"
}

Added a checkPermission function to check if the delegated account is authorized to send the transaction. And it can be extended in each sub-transactor to check the granular permissions.
the delegated account pays the fee
can query account_objects of the delegating account to see the permissions he owns.
A delegate transaction can be multi-signed by the delegated account's signer list

High Level Overview of Change

Context of Change

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Refactor (non-breaking change that only restructures code)
Performance (increase or change in throughput and/or latency)
Tests (you added tests for code that already exists, or your new feature included in this PR)
Documentation update
Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)
Release

API Impact

Public API: New feature (new methods and/or new fields)
Public API: Breaking change (in general, breaking changes should only impact the next api_version)
libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
Peer protocol change (must be backward compatible or bump the peer protocol version)

ximinez

Preliminary partial review. I know you're still working on this, but a few things jumped out at me, and I wanted to bring them to your attention before you go down a problematic path.

codecov · 2025-03-19T16:18:14Z

Codecov Report

Attention: Patch coverage is 99.55157% with 2 lines in your changes missing coverage. Please review.

Project coverage is 78.3%. Comparing base (9ec2d7f) to head (3839f1f).
Report is 1 commits behind head on develop.

Files with missing lines	Patch %	Lines
src/libxrpl/protocol/Permissions.cpp	97.7%	1 Missing ⚠️
src/xrpld/app/tx/detail/SetAccount.cpp	97.1%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff            @@
##           develop   #5354     +/-   ##
=========================================
+ Coverage     78.1%   78.3%   +0.1%     
=========================================
  Files          795     800      +5     
  Lines        68653   69025    +372     
  Branches      8276    8275      -1     
=========================================
+ Hits         53644   54022    +378     
+ Misses       15009   15003      -6

Files with missing lines	Coverage Δ
include/xrpl/protocol/ErrorCodes.h	`100.0% <ø> (ø)`
include/xrpl/protocol/Indexes.h	`100.0% <ø> (ø)`
include/xrpl/protocol/detail/ledger_entries.macro	`100.0% <100.0%> (ø)`
include/xrpl/protocol/detail/permissions.macro	`100.0% <100.0%> (ø)`
include/xrpl/protocol/detail/transactions.macro	`100.0% <100.0%> (ø)`
src/libxrpl/protocol/ErrorCodes.cpp	`85.7% <ø> (ø)`
src/libxrpl/protocol/Indexes.cpp	`98.1% <100.0%> (+<0.1%)`	⬆️
src/libxrpl/protocol/InnerObjectFormats.cpp	`100.0% <100.0%> (ø)`
src/libxrpl/protocol/STInteger.cpp	`86.4% <100.0%> (+1.9%)`	⬆️
src/libxrpl/protocol/STParsedJSON.cpp	`70.0% <100.0%> (+0.8%)`	⬆️
... and 20 more

... and 3 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

shawnxie999 · 2025-03-20T17:08:22Z

Also, I propose that we shouldn't need to squash commits. Commits makes it easier for the reviewers to keep track of the changes, and they will all be squashed in the end anyways. Also, the audit relies on a particular commit hash, so it's best to keep the commit history as authentic as possible.

gregtatcam

👍 LGTM

I have a few minor comments for your consideration,

ximinez

It looks like all my concerns have been addressed. I have not reviewed the whole PR, so I can not "approve", but I will dismiss my stale change request review.

As noted above, dismissing this change request.

tequdev · 2025-05-20T09:26:42Z

+    if (tx.isFieldPresent(sfWalletLocator) ||
+        tx.isFieldPresent(sfNFTokenMinter))
+        return tecNO_PERMISSION;


Isn't it necessary to check the WalletSize field?

The WalletSize field does nothing according to https://xrpl.org/docs/references/protocol/transactions/types/accountset
Although WalletSize is defined, there's no code in AccountSet setting WalletSize, which approves that the field is not used.

Yes, WalletSize does not change AccountRoot, but it is a field that can be specified in AccountSet transactions.
It seems strange that a transaction would succeed when only sfWaletSize is specified by an account with permissions only for AccountEmailHashSet.

Ah, if we have some specific permission for AccountSet, are AccountSet transactions (empty execution) that don't change AccountRoot allowed?

tequdev · 2025-05-20T09:30:44Z

+    if (uSetFlag != 0 || uClearFlag != 0 || uTxFlags != tfFullyCanonicalSig)
+        return tecNO_PERMISSION;


@dangell7

The tfInnerBatchTxn flag should be enabled here in Batch.
#5060

possibly uTxFlags & tfUniversalMask

The check now already blocks batch transaction and returns tecNO_PERMISSION. If the tx flag is not equal to the auto filled tfFullyCanonicalSig, it will return tecNO_PERMISSION. I think we don't need to modify for Batch.

Good Catch! @tequdev! You are right, as the current code stands Batch Inner AccountSet for Domain is rejected with NO_PERMISSION. tfUniversalMask should have been used here.

ok. Got it. So we are supporting delegation within the Batch transactions, even though Batch itself could not be delegated. Please ignore my previous comment. @dangell7 Thank you for updating.

## High Level Overview of Change  Adds support for Account Permission Delegation ([XLS-74d, XLS-75d](https://github.com/XRPLF/XRPL-Standards/pull/257/files)) Changes will include: - Adding a new transaction `DelegateSet` - Adding a common optional field `Delegate` to `SimpleTab` ### Context of Change  rippled PR: XRPLF/rippled#5354. ### Type of Change  - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] Refactor (non-breaking change that only restructures code) - [ ] Tests (You added tests for code that already exists, or your new feature included in this PR) - [ ] Documentation Updates - [ ] Translation Updates - [ ] Release ## Before / After  ### DelegateSet Simple Tab <img width="1008" alt="Screenshot 2025-05-21 at 3 17 53 PM" src="https://github.com/user-attachments/assets/f1058289-25c8-4f3d-8f7c-513354d58406" /> ### DelegateSet Table Detail <img width="1210" alt="Screenshot 2025-05-21 at 3 21 02 PM" src="https://github.com/user-attachments/assets/7ab86e54-379e-44f9-b6eb-f3269feac072" /> ### DelegateSet Description <img width="898" alt="Screenshot 2025-05-21 at 3 22 00 PM" src="https://github.com/user-attachments/assets/3542f6e3-7d1f-48c7-946a-87e8a7ba8fd3" /> ### An authorized Payment transaction Simple Tab <img width="1027" alt="Screenshot 2025-05-21 at 3 23 08 PM" src="https://github.com/user-attachments/assets/87deee32-fd75-4a06-ab55-56c341495eb0" /> ### An authorized Payment transaction Description <img width="719" alt="Screenshot 2025-05-21 at 3 23 29 PM" src="https://github.com/user-attachments/assets/f1d69437-cd4b-404a-b679-ccc14dfa3754" />

This change implements the account permission delegation described in XLS-75d, see XRPLF/XRPL-Standards#257. * Introduces transaction-level and granular permissions that can be delegated to other accounts. * Adds `DelegateSet` transaction to grant specified permissions to another account. * Adds `ltDelegate` ledger object to maintain the permission list for delegating/delegated account pair. * Adds an optional `Delegate` field in common fields, allowing a delegated account to send transactions on behalf of the delegating account within the granted permission scope. The `Account` field remains the delegating account; the `Delegate` field specifies the delegated account. The transaction is signed by the delegated account.

yinyiqian1 force-pushed the account_permission_new branch 2 times, most recently from 70a6362 to 5b262d1 Compare March 19, 2025 06:37

yinyiqian1 changed the title ~~Support AccountPermission~~ Support AccountPermission XLS-74d XLS-75d Mar 19, 2025

yinyiqian1 force-pushed the account_permission_new branch 5 times, most recently from c8bc5f6 to e8e74f1 Compare March 19, 2025 15:57

ximinez requested changes Mar 19, 2025

View reviewed changes

ximinez previously requested changes Mar 19, 2025

View reviewed changes

Comment thread src/libxrpl/protocol/Permissions.cpp Outdated

mvadari reviewed Mar 19, 2025

View reviewed changes

Comment thread include/xrpl/protocol/Permissions.h Outdated

Comment thread include/xrpl/protocol/Permissions.h Outdated

Comment thread src/libxrpl/protocol/Permissions.cpp Outdated

Comment thread src/xrpld/rpc/handlers/LedgerEntry.cpp Outdated

yinyiqian1 mentioned this pull request Mar 19, 2025

Support AccountPermission XLS-74d and XLS-75d #5164

Closed

13 tasks

yinyiqian1 force-pushed the account_permission_new branch from e8e74f1 to b1f0387 Compare March 20, 2025 14:51

shawnxie999 reviewed Mar 20, 2025

View reviewed changes

yinyiqian1 force-pushed the account_permission_new branch from b1f0387 to 4495956 Compare March 20, 2025 15:41

Support AccountPermission

49ca2b8

yinyiqian1 force-pushed the account_permission_new branch from 4495956 to 49ca2b8 Compare March 20, 2025 16:38

yinyiqian1 changed the title ~~Support AccountPermission XLS-74d XLS-75d~~ Support DelegateSet Amendment XLS-74d XLS-75d Mar 20, 2025

yinyiqian1 marked this pull request as ready for review March 20, 2025 16:50

shawnxie999 reviewed Mar 20, 2025

View reviewed changes

Comment thread src/xrpld/rpc/handlers/LedgerEntry.cpp Outdated

shawnxie999 reviewed Mar 20, 2025

View reviewed changes

Comment thread include/xrpl/protocol/Protocol.h Outdated

shawnxie999 reviewed Mar 20, 2025

View reviewed changes

Comment thread include/xrpl/protocol/detail/sfields.macro Outdated

mvadari reviewed Mar 20, 2025

View reviewed changes

Comment thread include/xrpl/protocol/detail/permissions.macro Outdated

mvadari reviewed Mar 20, 2025

View reviewed changes

Comment thread src/xrpld/app/tx/detail/DelegateSet.cpp Outdated

yinyiqian1 requested a review from gregtatcam March 20, 2025 18:20

shawnxie999 reviewed Mar 20, 2025

View reviewed changes

Comment thread src/xrpld/app/tx/detail/SetAccount.cpp Outdated

shawnxie999 reviewed Mar 20, 2025

View reviewed changes

Comment thread src/libxrpl/protocol/Permissions.cpp Outdated

shawnxie999 reviewed Mar 20, 2025

View reviewed changes

Comment thread src/xrpld/app/tx/detail/SetTrust.cpp Outdated

fix (void)enforce

3d245c8

gregtatcam approved these changes Apr 10, 2025

View reviewed changes

Comment thread include/xrpl/protocol/jss.h Outdated

Comment thread include/xrpl/protocol/Permissions.h Outdated

Comment thread src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp

Comment thread src/xrpld/app/misc/DelegateUtils.h

yinyiqian1 added 2 commits April 11, 2025 13:43

resolve comments

647ae72

Merge branch 'develop' into account_permission_new

5210035

kennyzlei mentioned this pull request Apr 22, 2025

Support PermissionDelegation (aka Account Permissions) XRPLF/clio#2015

Closed

kennyzlei added the Clio Reviewed label Apr 22, 2025

yinyiqian1 added 2 commits April 30, 2025 13:25

Merge branch 'develop' into account_permission_new

3d5aef6

add rpcDELEGATE_ACT_NOT_FOUND

0d80686

This was referenced Apr 30, 2025

Transaction model, unit and integ tests for Delegate XLS-74d amendment XRPLF/xrpl-py#833

Closed

XLS-74d and XLS-75d: Account Permissions XRPLF/XRPL-Standards#257

Merged

Add sequence test case

82f64d1

yinyiqian1 mentioned this pull request May 2, 2025

Delegating Account Permissions ripple/opensource.ripple.com#149

Merged

yinyiqian1 added 3 commits May 5, 2025 20:01

modify trustset granular unit test

7c853ec

Merge branch 'develop' into account_permission_new

a6f7e0c

Fix flag default value for AccountSet

6110424

ximinez reviewed May 7, 2025

View reviewed changes

kennyzlei added the Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. label May 7, 2025

clean up unit test feature passing

3839f1f

bthomee merged commit 2db2791 into XRPLF:develop May 8, 2025

Patel-Raj11 mentioned this pull request May 8, 2025

Implement account permission delegation - XLS-74d and XLS-75d amendments XRPLF/xrpl-py#840

Merged

9 tasks

khancode mentioned this pull request May 9, 2025

feat: add Account Permission Delegation (XLS-74d, XLS-75d) XRPLF/xrpl.js#2981

Merged

9 tasks

tequdev reviewed May 20, 2025

View reviewed changes

pdp2121 mentioned this pull request May 21, 2025

feat: support Account Permission Delegation (XLS-74d, XLS-75d) ripple/explorer#1178

Merged

8 tasks

Bronek mentioned this pull request Jun 24, 2025

Release 2.5.0 #5504

Merged

yinyiqian1 deleted the account_permission_new branch July 25, 2025 14:47

		if (uSetFlag != 0 \|\| uClearFlag != 0 \|\| uTxFlags != tfFullyCanonicalSig)
		return tecNO_PERMISSION;

Uh oh!

Conversation

yinyiqian1 commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

High Level Overview of Change

Context of Change

Type of Change

API Impact

Uh oh!

ximinez left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

codecov Bot commented Mar 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

shawnxie999 commented Mar 20, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gregtatcam left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ximinez left a comment

Choose a reason for hiding this comment

Uh oh!

tequdev May 20, 2025

Choose a reason for hiding this comment

Uh oh!

yinyiqian1 May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tequdev May 21, 2025

Choose a reason for hiding this comment

Uh oh!

tequdev May 21, 2025

Choose a reason for hiding this comment

Uh oh!

tequdev May 20, 2025

Choose a reason for hiding this comment

Uh oh!

yinyiqian1 May 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dangell7 May 20, 2025

Choose a reason for hiding this comment

Uh oh!

yinyiqian1 May 20, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

yinyiqian1 commented Mar 19, 2025 •

edited

Loading

ximinez left a comment •

edited

Loading

codecov Bot commented Mar 19, 2025 •

edited

Loading

yinyiqian1 May 20, 2025 •

edited

Loading

yinyiqian1 May 20, 2025 •

edited

Loading