[TA-4886] Add Account Permission Delegation (XLS-74d, XLS-75d)#144
[TA-4886] Add Account Permission Delegation (XLS-74d, XLS-75d)#144GuillemGarciaDev merged 21 commits intov0.1.xfrom
Conversation
…ebsocket and RPC client
…ermissions, and PermissionValue types
📝 WalkthroughWalkthroughThis change introduces delegated permissions functionality to the XRPL Go client and codec. It adds new types for granular and delegatable permissions, implements the DelegateSet transaction and Delegate ledger entry, and provides bidirectional mapping between permission names and values. Integration tests, serialization logic, and example programs for delegated transaction flows are included. Changes
Sequence Diagram(s)sequenceDiagram
participant Delegator as Delegator Wallet
participant Delegatee as Delegatee Wallet
participant XRPL as XRPL Network
Delegator->XRPL: Submit DelegateSet (authorize Delegatee, permissions)
XRPL-->>Delegator: DelegateSet validated and confirmed
Delegatee->XRPL: Submit Payment (as delegate, using delegated permission)
XRPL-->>Delegatee: Payment validated and confirmed
Suggested reviewers
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
… handle out-of-range errors
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (7)
binary-codec/definitions/definitions.go (1)
121-147: Consider using constants for hardcoded permission values.The hardcoded permission values (65537, 65538, etc.) should be defined as named constants to improve maintainability and avoid magic numbers. This would make the code more self-documenting and easier to maintain.
+const ( + // Granular permission base value + PermissionBaseValue = 65537 + + // Granular permission offsets + TrustlineAuthorizeOffset = 0 + TrustlineFreezeOffset = 1 + TrustlineUnfreezeOffset = 2 + // ... other offsets +) func initializePermissions() { definitions.GranularPermissions = map[string]int32{ - "TrustlineAuthorize": 65537, - "TrustlineFreeze": 65538, + "TrustlineAuthorize": PermissionBaseValue + TrustlineAuthorizeOffset, + "TrustlineFreeze": PermissionBaseValue + TrustlineFreezeOffset,xrpl/transaction/types/permission.go (1)
33-43: Consider enhancing permission value validation.The current validation only checks if the string is not empty. Depending on requirements, you might want to validate against known permission names to catch typos or invalid permissions early.
+import ( + "github.com/Peersyst/xrpl-go/binary-codec/definitions" +) // IsValid validates the PermissionValue structure. func (pv *PermissionValue) IsValid() bool { - return pv.PermissionValue != "" + if pv.PermissionValue == "" { + return false + } + + // Optionally validate against known permission names + defs := definitions.Get() + _, err := defs.GetDelegatablePermissionValueByName(pv.PermissionValue) + return err == nil }This would provide stronger validation by ensuring the permission name is actually valid, but the current implementation may be sufficient if validation happens elsewhere.
CHANGELOG.md (2)
18-19: Documentation accurately reflects new features.The changelog entries properly document the addition of permission-related definitions to the binary codec. However, consider varying the sentence structure to improve readability, as suggested by the static analysis tool.
Consider rephrasing to reduce repetitive "Adds" beginnings:
- Adds `GranularPermissions` and `DelegatablePermissions` entries to definitions. - Adds `PermissionValue` serialized type with custom serializer routing. + Adds `GranularPermissions` and `DelegatablePermissions` entries to definitions. + Introduces `PermissionValue` serialized type with custom serializer routing.
28-31: Comprehensive documentation of delegated permissions features.The changelog accurately documents all major components of the delegated permissions implementation, including proper XLS standard references. The content is comprehensive and technically accurate.
Consider varying sentence structure here as well:
- Adds `DelegateSet` transaction type (XLS-74d) with validation and error support. - Adds `Delegate` ledger entry type (XLS-74d). - Adds `PermissionValue` and `Permission` types for delegated permissions. - Adds integration tests for `DelegateSet` submission and delegated `Payment` execution (XLS-75d). + Adds `DelegateSet` transaction type (XLS-74d) with validation and error support. + Introduces `Delegate` ledger entry type (XLS-74d). + Implements `PermissionValue` and `Permission` types for delegated permissions. + Includes integration tests for `DelegateSet` submission and delegated `Payment` execution (XLS-75d).binary-codec/types/permission_value.go (1)
25-77: Comprehensive input type handling with proper validation.The
FromJSONmethod correctly handles string-to-permission conversion and validates numeric inputs. The range checking and big-endian encoding are appropriate for XRPL serialization.Consider adding precision checking for float64 to avoid potential truncation issues:
case float64: - if v < 0 || v > float64(math.MaxUint32) { + if v < 0 || v > float64(math.MaxUint32) || v != math.Trunc(v) { return nil, ErrPermissionValueOutOfRange }examples/delegate-set/ws/main.go (1)
149-162: Improve error handling in getAccountBalance.Returning "0" on error without any indication could mask issues. Consider logging the error or returning a different sentinel value.
Apply this diff to improve error visibility:
func getAccountBalance(client *websocket.Client, address types.Address) string { accountInfo, err := client.GetAccountInfo(&account.InfoRequest{ Account: address, }) if err != nil { - return "0" + fmt.Printf("Warning: Failed to get balance for %s: %v\n", address, err) + return "unknown" }xrpl/transaction/delegate_set.go (1)
112-119: Consider using a more specific error for the Authorize field.Using
ErrInvalidDestinationfor the Authorize field validation might be confusing since it's not a destination field. Consider defining a specific error likeErrDelegateSetInvalidAuthorize.Add a new error definition at the top of the file:
// ErrDelegateSetInvalidAuthorize is returned when the Authorize field is missing or invalid. ErrDelegateSetInvalidAuthorize = errors.New("authorize field is required and must be a valid address")Then update the validation:
- if d.Authorize == "" { - return false, ErrInvalidDestination // Using existing error for required address field - } - - if !addresscodec.IsValidAddress(d.Authorize.String()) { - return false, ErrInvalidDestination - } + if d.Authorize == "" { + return false, ErrDelegateSetInvalidAuthorize + } + + if !addresscodec.IsValidAddress(d.Authorize.String()) { + return false, ErrDelegateSetInvalidAuthorize + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (26)
CHANGELOG.md(2 hunks)binary-codec/definitions/definitions.go(3 hunks)binary-codec/definitions/definitions.json(6 hunks)binary-codec/definitions/definitions_test.go(1 hunks)binary-codec/definitions/helpers.go(1 hunks)binary-codec/definitions/helpers_test.go(1 hunks)binary-codec/types/permission_value.go(1 hunks)binary-codec/types/permission_value_test.go(1 hunks)binary-codec/types/serialized_type.go(1 hunks)binary-codec/types/serialized_type_test.go(1 hunks)binary-codec/types/st_array.go(1 hunks)binary-codec/types/st_object.go(2 hunks)examples/delegate-set/rpc/main.go(1 hunks)examples/delegate-set/ws/main.go(1 hunks)xrpl/ledger-entry-types/delegate.go(1 hunks)xrpl/ledger-entry-types/delegate_test.go(1 hunks)xrpl/ledger-entry-types/ledger_object.go(3 hunks)xrpl/transaction/delegate_set.go(1 hunks)xrpl/transaction/delegate_set_test.go(1 hunks)xrpl/transaction/errors.go(1 hunks)xrpl/transaction/integration/delegate_set_test.go(1 hunks)xrpl/transaction/tx.go(3 hunks)xrpl/transaction/tx_test.go(3 hunks)xrpl/transaction/tx_type.go(1 hunks)xrpl/transaction/types/permission.go(1 hunks)xrpl/transaction/types/permission_test.go(1 hunks)
🧰 Additional context used
🧠 Learnings (15)
📓 Common learnings
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: vriveraPeersyst
PR: Peersyst/xrpl-snap#129
File: packages/site/src/domain/transaction/controller/TransactionController.ts:98-111
Timestamp: 2025-05-17T14:29:55.132Z
Learning: In the TransactionController class of the XRPL Snap project, the `...rest` parameter in the `sendIOUTransaction` method does not contain flags from callers, so directly setting `flags: PARTIAL_PAYMENT_FLAG` without merging with existing flags is appropriate.
xrpl/transaction/tx_type.go (2)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/ledger-entry-types/ledger_object.go (3)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/transaction/errors.go (3)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/transaction/tx.go (3)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
xrpl/transaction/types/permission.go (4)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: GuillemGarciaDev
PR: Peersyst/xrpl-go#131
File: xrpl/rpc/client.go:265-265
Timestamp: 2025-03-31T17:29:49.615Z
Learning: In the XRPL Go codebase, type assertions from maps should always use the two-return pattern (value, ok := map[key].(type)) with proper error handling to avoid potential panics.
xrpl/ledger-entry-types/delegate_test.go (4)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: GuillemGarciaDev
PR: Peersyst/xrpl-go#131
File: xrpl/rpc/client.go:265-265
Timestamp: 2025-03-31T17:29:49.615Z
Learning: In the XRPL Go codebase, type assertions from maps should always use the two-return pattern (value, ok := map[key].(type)) with proper error handling to avoid potential panics.
xrpl/transaction/types/permission_test.go (1)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/ledger-entry-types/delegate.go (3)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
binary-codec/types/permission_value_test.go (1)
Learnt from: GuillemGarciaDev
PR: Peersyst/xrpl-go#131
File: xrpl/rpc/client.go:265-265
Timestamp: 2025-03-31T17:29:49.615Z
Learning: In the XRPL Go codebase, type assertions from maps should always use the two-return pattern (value, ok := map[key].(type)) with proper error handling to avoid potential panics.
xrpl/transaction/delegate_set_test.go (2)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
CHANGELOG.md (2)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/transaction/tx_test.go (3)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
xrpl/transaction/delegate_set.go (3)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
xrpl/transaction/integration/delegate_set_test.go (3)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
🧬 Code Graph Analysis (15)
binary-codec/types/serialized_type_test.go (4)
binary-codec/types/serialized_type.go (2)
SerializedType(14-17)GetSerializedTypeForField(66-74)binary-codec/types/uint32.go (1)
UInt32(11-11)binary-codec/types/amount.go (1)
Amount(101-101)binary-codec/types/account_id.go (1)
AccountID(12-12)
binary-codec/types/st_array.go (1)
binary-codec/types/serialized_type.go (1)
GetSerializedTypeForField(66-74)
binary-codec/definitions/helpers_test.go (1)
binary-codec/definitions/errors.go (2)
NotFoundError(18-21)NotFoundErrorInt(29-32)
xrpl/ledger-entry-types/ledger_object.go (1)
xrpl/ledger-entry-types/delegate.go (2)
Delegate(23-45)Delegate(48-50)
binary-codec/definitions/definitions.go (1)
binary-codec/definitions/field_instance.go (1)
FieldHeader(23-26)
xrpl/transaction/tx.go (2)
address-codec/codec.go (1)
IsValidAddress(181-183)xrpl/transaction/errors.go (2)
ErrInvalidDelegate(13-13)ErrDelegateAccountConflict(15-15)
binary-codec/types/st_object.go (1)
binary-codec/types/serialized_type.go (1)
GetSerializedTypeForField(66-74)
binary-codec/definitions/helpers.go (2)
binary-codec/definitions/definitions.go (1)
Definitions(17-26)binary-codec/definitions/errors.go (2)
NotFoundError(18-21)NotFoundErrorInt(29-32)
xrpl/ledger-entry-types/delegate_test.go (5)
xrpl/ledger-entry-types/delegate.go (2)
Delegate(23-45)Delegate(48-50)xrpl/ledger-entry-types/ledger_object.go (2)
EntryType(8-8)DelegateEntry(17-17)binary-codec/types/hash256.go (1)
Hash256(4-6)xrpl/transaction/types/permission.go (1)
Permission(12-14)xrpl/testutil/serializer.go (1)
SerializeAndDeserialize(34-39)
xrpl/transaction/types/permission_test.go (1)
xrpl/transaction/types/permission.go (2)
Permission(12-14)PermissionValue(17-19)
xrpl/ledger-entry-types/delegate.go (3)
binary-codec/types/hash256.go (1)
Hash256(4-6)xrpl/ledger-entry-types/ledger_object.go (2)
EntryType(8-8)DelegateEntry(17-17)xrpl/transaction/types/permission.go (1)
Permission(12-14)
xrpl/transaction/delegate_set_test.go (4)
xrpl/transaction/delegate_set.go (2)
DelegateSet(71-77)DelegateSet(80-82)xrpl/transaction/tx_type.go (2)
DelegateSetTx(23-23)TxType(3-3)xrpl/transaction/tx.go (1)
BaseTx(30-103)xrpl/transaction/types/permission.go (2)
Permission(12-14)PermissionValue(17-19)
xrpl/transaction/tx_test.go (2)
xrpl/transaction/tx.go (1)
BaseTx(30-103)xrpl/transaction/tx_type.go (1)
PaymentTx(47-47)
examples/delegate-set/rpc/main.go (6)
xrpl/rpc/config.go (1)
WithFaucetProvider(57-61)xrpl/faucet/devnet.go (1)
NewDevnetFaucetProvider(26-31)xrpl/wallet/wallet.go (2)
New(38-44)Wallet(29-34)pkg/crypto/ed25519.go (1)
ED25519(30-34)xrpl/transaction/tx.go (1)
BaseTx(30-103)xrpl/transaction/types/permission.go (1)
Permission(12-14)
examples/delegate-set/ws/main.go (7)
xrpl/faucet/devnet.go (1)
NewDevnetFaucetProvider(26-31)xrpl/wallet/wallet.go (1)
New(38-44)pkg/crypto/ed25519.go (1)
ED25519(30-34)xrpl/transaction/tx.go (1)
BaseTx(30-103)xrpl/transaction/types/permission.go (1)
Permission(12-14)xrpl/queries/common/ledger.go (1)
Validated(59-59)xrpl/queries/account/v1/types/account_data.go (1)
AccountData(5-8)
🪛 LanguageTool
CHANGELOG.md
[style] ~18-~18: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...s unit tests for MPTCurrencyAmount. - Adds GranularPermissions and `DelegatableP...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~19-~19: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ePermissionsentries to definitions. - AddsPermissionValue` serialized type with ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~28-~28: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...dds NFTokenModify transaction type. - Adds DelegateSet transaction type (XLS-74d...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~29-~29: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d) with validation and error support. - Adds Delegate ledger entry type (XLS-74d)....
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~30-~30: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...elegateledger entry type (XLS-74d). - AddsPermissionValueandPermission` type...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~31-~31: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...iontypes for delegated permissions. - Adds integration tests forDelegateSet` sub...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (53)
binary-codec/definitions/definitions_test.go (1)
29-30: Approved: Permission test values verified
- Confirmed
definitions.GranularPermissions["TrustlineAuthorize"]is set to 65537 in definitions.go.- Confirmed
definitions.TransactionTypes["Payment"]is 0 in definitions.json, soDelegatablePermissions["Payment"]is computed as 0 + 1 = 1.Tests correctly cover the new permission definitions.
xrpl/transaction/tx_type.go (1)
23-23: LGTM! New transaction type constant added correctly.The
DelegateSetTxconstant follows the established naming pattern and is properly placed in alphabetical order.binary-codec/types/st_object.go (2)
43-43: LGTM! Updated to use field-specific type resolution.The change to
GetSerializedTypeForFieldenables proper handling of the newPermissionValuefield while maintaining backward compatibility.
73-73: LGTM! Consistent update for ToJSON method.The change mirrors the update in
FromJSONmethod, ensuring consistent field-specific type resolution for both serialization and deserialization.binary-codec/types/st_array.go (1)
74-74: LGTM! Consistent field-specific type resolution for arrays.The change aligns with the updates in
st_object.goand ensures thatPermissionValuefields are properly handled within STArray structures.binary-codec/types/serialized_type.go (1)
64-74: LGTM! Well-implemented field-specific type resolution.The new
GetSerializedTypeForFieldfunction provides a clean solution for handling thePermissionValuefield while maintaining backward compatibility through the fallback toGetSerializedType.xrpl/transaction/errors.go (1)
12-15: LGTM! Well-structured error definitions for delegate validation.The new error constants follow the established naming patterns and provide clear, descriptive messages. The placement alongside similar address validation errors maintains good organization.
binary-codec/types/serialized_type_test.go (1)
106-151: Excellent test coverage for the new field-specific type resolution.The test thoroughly validates the special handling of
PermissionValuefields while ensuring other field types continue to use standard type resolution. The table-driven approach and comprehensive test cases including edge cases demonstrate good testing practices.xrpl/transaction/tx.go (3)
57-60: Well-documented Delegate field addition.The field definition follows established patterns with proper documentation, appropriate type, and correct JSON omitempty tag for optional fields.
165-167: Correct flattening implementation for Delegate field.The Delegate field is properly added to the flattened transaction map when present, following the same pattern as other optional address fields.
227-236: Robust validation logic for Delegate field.The validation implementation correctly:
- Only validates when Delegate is set (avoiding unnecessary checks)
- Uses the same address validation as other address fields
- Prevents the important business rule violation of Delegate == Account
- Returns appropriate error constants for clear error reporting
xrpl/transaction/tx_test.go (2)
128-163: Comprehensive test coverage for Delegate field validation.The three new test cases thoroughly validate the Delegate field functionality:
- Valid delegate transaction (happy path)
- Invalid delegate address (error handling)
- Delegate-Account conflict prevention (business rule enforcement)
These tests ensure the validation logic works correctly across all scenarios.
211-211: Proper test updates for Delegate field flattening.The flatten test correctly includes the Delegate field in both the input transaction and expected JSON output, ensuring the flattening functionality works as expected.
Also applies to: 236-236
xrpl/ledger-entry-types/ledger_object.go (3)
17-17: Proper addition of DelegateEntry constant.The constant follows the established naming convention and is positioned correctly in alphabetical order with other entry types.
61-62: Correct factory method implementation for Delegate objects.The EmptyLedgerObject function properly handles DelegateEntry by returning a new Delegate instance, following the same pattern as other ledger entry types.
124-125: Appropriate unmarshaling support for Delegate entries.The UnmarshalLedgerObject function correctly creates a Delegate instance for unmarshaling, maintaining consistency with the handling of other ledger entry types.
binary-codec/definitions/helpers_test.go (2)
608-650: Well-structured test implementation for permission lookups.The test function follows the established table-driven test pattern used throughout the file and provides comprehensive coverage for both valid and invalid permission names. The error handling correctly validates the specific error types returned.
652-694: Comprehensive test coverage for reverse permission lookup.The test properly validates the bidirectional lookup functionality by testing both granular permissions and transaction type permissions. The error handling for invalid values correctly expects
NotFoundErrorIntwith proper instance and input values.binary-codec/definitions/helpers.go (2)
201-212: Consistent implementation following established patterns.The method correctly follows the same pattern as other lookup functions in this file, with proper error handling and documentation. The use of
DelegatablePermissionsmap andNotFoundErroris consistent with the codebase conventions.
214-226: Proper reverse lookup implementation with consistent error handling.The method correctly iterates through the
DelegatablePermissionsmap to find the matching value and returns the appropriateNotFoundErrorIntwhen no match is found. The implementation follows the same pattern as other reverse lookup methods in the file.binary-codec/definitions/definitions.go (2)
24-25: New permission maps correctly added to Definitions struct.The addition of
GranularPermissionsandDelegatablePermissionsmaps is consistent with the existing struct design and supports the new delegated permissions functionality.
144-146: No numeric conflicts between transaction delegations and granular permissions
- Max transaction type code = 102 → delegatable permission = 103
- Granular permissions start at 65537
- Ranges are disjoint, so no overlap
xrpl/ledger-entry-types/delegate_test.go (2)
11-14: Simple and effective EntryType test.The test correctly verifies that the
EntryType()method returns the expectedDelegateEntryconstant, ensuring proper type identification for the new ledger entry type.
16-113: Comprehensive serialization test with good coverage.The test provides excellent coverage by testing both full and minimal
Delegateconfigurations. The JSON expectations are well-formatted and include realistic XRPL addresses and hash values. The use oftestutil.SerializeAndDeserializeensures proper round-trip serialization validation.xrpl/transaction/types/permission.go (2)
5-8: Clear error definition for permission validation.The error variable is well-defined with a descriptive message that clearly indicates the validation failure condition.
10-31: Well-structured Permission type with proper validation.The
Permissionstruct correctly wraps thePermissionValueand delegates validation and flattening to the inner type. The structure matches the described xrpl.js interface and provides clean method implementations.binary-codec/definitions/definitions.json (6)
44-44: Correct addition of Delegate ledger entry type.The new
Delegateledger entry type is properly defined with an appropriate numeric identifier (131) that follows the sequential pattern of existing entries.
3120-3120: DelegateSet transaction type properly added.The
DelegateSettransaction type is correctly defined with value 64, which appropriately follows the sequence of existing transaction types.
878-887: PermissionValue nth index verified uniqueThe
PermissionValuefield usesUInt32withnth: 52, and no other field inbinary-codec/definitions/definitions.jsonshares this index.• Location:
binary-codec/definitions/definitions.json(lines 878–887)
• Field:"PermissionValue"–type: "UInt32",nth: 52All metadata flags are correctly set.
188-197: Permissions STArray ‘nth’ value is uniqueValidation confirms no other
STArrayentry usesnth: 29. The field definition is correct as is.
- File:
binary-codec/definitions/definitions.json(lines 188–197)
2838-2847: Confirmed unique nth value for Delegate AccountID fieldVerification shows no other
AccountIDfields usenth: 12inbinary-codec/definitions/definitions.json. TheDelegatefield is correctly defined and requires no changes.
2488-2497: Permission nth index verified: no conflicts detectedNo other STObject entries use
nth: 15. The newPermissionfield is correctly defined and ready for merge.
- File: binary-codec/definitions/definitions.json (lines 2488–2497)
examples/delegate-set/rpc/main.go (6)
1-25: Well-structured example setup.The imports, client configuration, and error handling are appropriate for a demonstration program. Using the devnet environment is correct for examples.
28-49: Proper wallet creation and funding logic.The example correctly creates ED25519 wallets and funds them using the devnet faucet. Error handling is appropriate for demonstration purposes.
51-63: Good balance checking with graceful error handling.The initial balance retrieval logic handles errors appropriately by defaulting to "0", which prevents the example from failing due to temporary network issues while still providing useful information.
65-94: Correct DelegateSet transaction implementation.The DelegateSet transaction is properly constructed with the appropriate permission structure. The use of
SubmitTxAndWaitwith autofill is suitable for this demonstration.
98-122: Excellent demonstration of delegated payment.The delegated payment transaction correctly shows how to use the new delegation feature by setting the
Delegatefield and signing with the delegatee wallet. The 3-second wait allows time for the delegation to be processed.
123-135: Consistent final balance display.The final balance checking maintains the same error handling pattern as the initial balance check, providing users with clear before/after comparison of the delegation example.
binary-codec/types/permission_value.go (2)
1-21: Well-defined error types and proper imports.The error definitions are specific and informative, and the imports include all necessary packages for binary encoding and permission definitions. The minimal struct definition is appropriate for a serialization type.
82-96: Correct reverse serialization with appropriate fallback.The
ToJSONmethod properly reads the binary data and attempts to convert back to permission names. The fallback to numeric values when names aren't found ensures compatibility with unknown permission values.xrpl/ledger-entry-types/delegate.go (3)
1-22: Excellent documentation with practical example.The comprehensive documentation includes a helpful JSON example and properly mentions the required AccountPermissionDelegation amendment. This provides clear guidance for developers.
23-45: Complete and well-documented struct definition.The
Delegatestruct includes all necessary fields for delegation functionality with appropriate types and clear field documentation. The JSON tags follow established conventions.
47-50: Correct EntryType method implementation.The
EntryType()method correctly returns theDelegateEntryconstant, following the established pattern for ledger entry types.binary-codec/types/permission_value_test.go (1)
1-173: Well-structured and comprehensive test coverage!The tests thoroughly cover various input types, error scenarios, and edge cases for PermissionValue serialization. Good use of table-driven tests and mocking.
xrpl/transaction/delegate_set_test.go (1)
1-352: Excellent test coverage for DelegateSet transaction!The tests comprehensively cover all validation scenarios, edge cases, and error conditions. The table-driven approach makes it easy to add new test cases.
xrpl/transaction/integration/delegate_set_test.go (1)
118-118: ErrInvalidTransaction is locally defined; no prefix needed.The constant
ErrInvalidTransactionis declared inxrpl/transaction/integration/types.go, and sincedelegate_set_test.gois in the sameintegrationpackage, unqualified references resolve correctly. No import or prefix is required—this will compile as is.Likely an incorrect or invalid review comment.
xrpl/transaction/delegate_set.go (7)
1-8: LGTM!The import statements are appropriate for the functionality being implemented.
10-24: LGTM!The error definitions are well-structured with clear, descriptive messages and follow Go naming conventions.
26-42: LGTM!The constants are well-defined. The non-delegatable transactions list appropriately includes sensitive operations that should not be delegated.
44-77: LGTM!The struct is well-designed with clear documentation and a helpful JSON example.
79-82: LGTM!Standard implementation for the transaction type method.
84-103: LGTM!The Flatten method correctly handles the transaction serialization, including proper handling of optional fields and nested permission structures.
126-161: LGTM!The permissions validation logic is comprehensive and efficient, properly checking for array bounds, valid permission values, non-delegatable transactions, and duplicates.
banasa44
added
feature
…l value fields parsing
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
Makefile (1)
6-7: Use...wildcard to future-proof the package path
INTEGRATION_TEST_PACKAGESis set to a single directory path.
If sub-packages are ever introduced underxrpl/transaction/integration/, they will be silently skipped bygo test. Using the recursive pattern makes the target resilient:-INTEGRATION_TEST_PACKAGES = ./xrpl/transaction/integration +INTEGRATION_TEST_PACKAGES = ./xrpl/transaction/integration/...CHANGELOG.md (1)
24-39: Consider varying sentence beginnings for better readability.The changelog entries accurately document the new delegation features. However, consider varying the sentence beginnings to improve readability:
- Adds `GranularPermissions` and `DelegatablePermissions` entries to definitions. - Adds `PermissionValue` serialized type with custom serializer routing. + Adds `GranularPermissions` and `DelegatablePermissions` entries to definitions. + Introduces `PermissionValue` serialized type with custom serializer routing.- Adds `DelegateSet` transaction type (XLS-74d) with validation and error support. - Adds `Delegate` ledger entry type (XLS-74d). - Adds `PermissionValue` and `Permission` types for delegated permissions. - Adds integration tests for `DelegateSet` submission and delegated `Payment` execution (XLS-75d). + Adds `DelegateSet` transaction type (XLS-74d) with validation and error support. + Introduces `Delegate` ledger entry type (XLS-74d). + Provides `PermissionValue` and `Permission` types for delegated permissions. + Includes integration tests for `DelegateSet` submission and delegated `Payment` execution (XLS-75d).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
CHANGELOG.md(2 hunks)Makefile(2 hunks)binary-codec/definitions/definitions.json(6 hunks)binary-codec/types/st_object.go(4 hunks)examples/delegate-set/rpc/main.go(1 hunks)examples/delegate-set/ws/main.go(1 hunks)xrpl/ledger-entry-types/delegate_test.go(1 hunks)xrpl/transaction/delegate_set.go(1 hunks)xrpl/transaction/tx.go(3 hunks)xrpl/transaction/tx_test.go(3 hunks)xrpl/transaction/tx_type.go(1 hunks)xrpl/transaction/types/permission_test.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (8)
- xrpl/transaction/tx_type.go
- xrpl/transaction/tx.go
- binary-codec/types/st_object.go
- xrpl/transaction/tx_test.go
- examples/delegate-set/rpc/main.go
- xrpl/ledger-entry-types/delegate_test.go
- xrpl/transaction/types/permission_test.go
- binary-codec/definitions/definitions.json
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
examples/delegate-set/ws/main.go (5)
Learnt from: banasa44
PR: Peersyst/xrpl-go#144
File: examples/delegate-set/ws/main.go:26-28
Timestamp: 2025-07-04T08:04:15.312Z
Learning: In the xrpl-go codebase, example files consistently use early returns with fmt.Println(err) and return directly in main functions for error handling, while relying on defer statements like defer client.Disconnect() for cleanup. There's no pattern of wrapping main logic in separate functions to ensure cleanup - the defer statements handle this correctly with early returns.
Learnt from: GuillemGarciaDev
PR: Peersyst/xrpl-go#131
File: xrpl/rpc/client.go:265-265
Timestamp: 2025-03-31T17:29:49.615Z
Learning: In the XRPL Go codebase, type assertions from maps should always use the two-return pattern (value, ok := map[key].(type)) with proper error handling to avoid potential panics.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
xrpl/transaction/delegate_set.go (3)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: The CredentialCreate struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
CHANGELOG.md (2)
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In the PR #113 for xrpl-go, the initial flagging of duplicated fields (`CredentialType` and `Expiration`) in the `xrpl/transaction/credential_create.go` file was determined to be irrelevant and not an actual issue.
Learnt from: florent-uzio
PR: Peersyst/xrpl-go#113
File: xrpl/transaction/credential_create.go:30-34
Timestamp: 2025-03-11T11:50:58.330Z
Learning: In PR #113 for xrpl-go, the `CredentialCreate` struct in xrpl/transaction/credential_create.go does not have any duplicated fields as was incorrectly flagged in a previous review comment.
🧬 Code Graph Analysis (1)
xrpl/transaction/delegate_set.go (5)
xrpl/transaction/tx_type.go (6)
AccountSetTx(7-7)SetRegularKeyTx(54-54)SignerListSetTx(55-55)DelegateSetTx(24-24)AccountDeleteTx(8-8)TxType(3-3)xrpl/transaction/tx.go (1)
BaseTx(30-103)xrpl/transaction/types/permission.go (2)
Permission(12-14)PermissionValue(17-19)address-codec/codec.go (1)
IsValidAddress(181-183)xrpl/transaction/errors.go (1)
ErrInvalidDestination(23-23)
🪛 LanguageTool
CHANGELOG.md
[style] ~24-~24: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...s unit tests for MPTCurrencyAmount. - Adds GranularPermissions and `DelegatableP...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~25-~25: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...ePermissionsentries to definitions. - AddsPermissionValue` serialized type with ...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~36-~36: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...dds NFTokenModify transaction type. - Adds DelegateSet transaction type (XLS-74d...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~37-~37: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...d) with validation and error support. - Adds Delegate ledger entry type (XLS-74d)....
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~38-~38: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...elegateledger entry type (XLS-74d). - AddsPermissionValueandPermission` type...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~39-~39: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...iontypes for delegated permissions. - Adds integration tests forDelegateSet` sub...
(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
🔇 Additional comments (5)
examples/delegate-set/ws/main.go (1)
1-151: LGTM! Well-structured demonstration of delegated permissions.The example effectively demonstrates the new DelegateSet functionality by:
- Creating a DelegateSet transaction granting "Payment" permission to the delegatee
- Executing a delegated Payment transaction where the delegatee acts on behalf of the delegator
- Following established codebase patterns for error handling and client usage
The code properly shows both the permission granting and usage phases of the delegation workflow.
xrpl/transaction/delegate_set.go (4)
10-24: Well-defined error handling with comprehensive coverage.The error definitions provide clear, specific error messages for all validation scenarios. The naming convention follows Go best practices with descriptive names that clearly indicate the nature of each error.
31-42: Properly restricts non-delegatable transaction types.The NonDelegatableTransactionsMap correctly identifies sensitive transaction types that should not be delegatable (account management, key management, delegation management). The inclusion of pseudo transactions like "EnableAmendment", "SetFee", and "UNLModify" shows thorough consideration of the XRPL transaction ecosystem.
44-77: Excellent documentation and struct design.The DelegateSet struct is well-documented with:
- Clear field descriptions
- Comprehensive JSON example showing real-world usage
- Proper embedding of BaseTx for common transaction fields
- Appropriate JSON tags for serialization
The example JSON demonstrates the expected structure and helps developers understand how to use the transaction type.
105-156: Comprehensive validation logic with robust error handling.The validation method thoroughly covers all edge cases:
- Base transaction validation
- Address format validation using the established address codec
- Conflict prevention between Account and Authorize fields
- Permission array bounds checking
- Individual permission validation
- Non-delegatable transaction type prevention
- Duplicate permission detection
The duplicate detection logic using map length comparison is efficient and follows the JavaScript implementation pattern mentioned in the comment.
2 participants
[TA-4886] Add Account Permission Delegation (XLS-74d, XLS-75d)
Description
This PR adds end-to-end support for Account Permission Delegation (XLS-74d, XLS-75d).
Type of change
Checklist
Changes
binary-codec/definitions
binary-codec/types
xrpl/transaction
xrpl/ledger-entry-types
examples/delegate-set
integration tests
Changelog
Notes (optional)
Created a
PermissionValueserializer to avoid special-case code inUInt32. AddedGetSerializedTypeForField(fieldName, fieldType)so only thePermissionValuefield uses that serializer and everything else uses the default type lookup.Summary by CodeRabbit
New Features
Bug Fixes
Tests
Documentation