Skip to content

Add support for DomainID in MPTokenIssuance transactions #5509

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 15 commits into from
Jul 23, 2025

Conversation

Bronek
Copy link
Collaborator

@Bronek Bronek commented Jun 24, 2025

High Level Overview of Change

Add support for DomainID to existing transactions MPTokenIssuanceCreate and MPTokenIssuanceSet

Context of Change

In #5224 we added DomainID as an access control mechanism for SingleAssetVault. The actual implementation of this feature lies in MPToken and MPTokenIssuance, hence it makes sense to enable the use of DomainID also in MPTokenIssuanceCreate and MPTokenIssuanceSet, following same rules as in Vault:

  • MPTokenIssuanceCreate and MPTokenIssuanceSet can only set DomainID if flag MPTRequireAuth is set
  • MPTokenIssuanceCreate requires that DomainID be a non-zero, uint256 number
  • MPTokenIssuanceSet allows DomainID to be zero (or empty) in which case it will remove DomainID from the MPTokenIssuance object

The change is amendment-gated by SingleAssetVault.

This is a non-breaking change because SingleAssetVault amendment is Supported::no, i.e. at this moment considered a work in progress, which cannot be enabled on the network.

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

@Bronek Bronek requested a review from a team as a code owner June 24, 2025 15:57
@Bronek Bronek changed the title Add support for DomainID in MPTokenIssuance transactions Add support for DomainID in MPTokenIssuance transactions Jun 24, 2025
@Bronek Bronek mentioned this pull request Jun 24, 2025
13 tasks
Copy link

codecov bot commented Jun 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.8%. Comparing base (433eeab) to head (61105f9).
Report is 1 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##           develop   #5509   +/-   ##
=======================================
  Coverage     78.8%   78.8%           
=======================================
  Files          814     814           
  Lines        71163   71201   +38     
  Branches      8354    8331   -23     
=======================================
+ Hits         56042   56106   +64     
+ Misses       15121   15095   -26     
Files with missing lines Coverage Δ
include/xrpl/protocol/detail/ledger_entries.macro 100.0% <ø> (ø)
include/xrpl/protocol/detail/transactions.macro 100.0% <ø> (ø)
src/xrpld/app/tx/detail/MPTokenIssuanceCreate.cpp 96.2% <100.0%> (+2.0%) ⬆️
src/xrpld/app/tx/detail/MPTokenIssuanceSet.cpp 98.0% <100.0%> (+0.7%) ⬆️

... and 6 files with indirect coverage changes

Impacted file tree graph

🚀 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.

@Bronek Bronek force-pushed the Bronek/DomainID_to_MPTokenIssuance_txs branch from 300fe81 to b486cdf Compare June 25, 2025 16:01
@Bronek Bronek requested a review from gregtatcam June 25, 2025 16:16
@Bronek Bronek self-assigned this Jun 26, 2025
@Bronek Bronek requested a review from gregtatcam July 2, 2025 12:24
@Bronek Bronek force-pushed the Bronek/DomainID_to_MPTokenIssuance_txs branch from 13ae4bd to 87282f2 Compare July 2, 2025 12:35
Copy link
Collaborator

@gregtatcam gregtatcam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@Bronek Bronek requested a review from mvadari July 7, 2025 08:29
@vlntb vlntb self-requested a review July 7, 2025 13:03
Copy link
Collaborator

@vlntb vlntb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good.
Couple of nit comments. Happy to approve otherwise.

@Bronek Bronek added the Needs second review PR requires at least one more code review approval before it can be merged label Jul 23, 2025
@Bronek Bronek added Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required. and removed Needs second review PR requires at least one more code review approval before it can be merged labels Jul 23, 2025
@bthomee bthomee merged commit 80d82c5 into develop Jul 23, 2025
27 checks passed
@bthomee bthomee deleted the Bronek/DomainID_to_MPTokenIssuance_txs branch July 23, 2025 17:21
ximinez added a commit that referenced this pull request Jul 24, 2025
…to ximinez/lending-refactoring-3

* XRPLF/ximinez/lending-refactoring-2:
  refactor: Update rocksdb (#5568)
  Switch instrumentation workflow to use dependencies (#5607)
  chore: Rename conan profile to `default` (#5599)
  Include `network_id` in validations and subscription stream responses (#5579)
  Add support for `DomainID` in `MPTokenIssuance` transactions (#5509)
  chore: Remove unused code after flow cross retirement (#5575)
  Remove obsolete owner pays fee feature and XRPL_ABANDON stanza (#5550)
  refactor: Makes HashRouter flags more type-safe (#5371)
  Fix clang-format CI job (#5598)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready to merge *PR author* thinks it's ready to merge. Has passed code review. Perf sign-off may still be required.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants