Support for Dynamic MPTs (XLS-94D) #3081
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -122,8 +122,11 @@ jobs: | |
| fetch-depth: 0 | ||
|
|
||
| - name: Run docker in background | ||
| id: run-docker | ||
| run: | | ||
| docker run --detach --rm -p 6006:6006 --volume "${{ github.workspace }}/.ci-config/":"/etc/opt/ripple/" --name rippled-service --health-cmd="rippled server_info || exit 1" --health-interval=5s --health-retries=10 --health-timeout=2s --env GITHUB_ACTIONS=true --env CI=true --entrypoint bash ${{ env.RIPPLED_DOCKER_IMAGE }} -c "rippled -a" | ||
| CONTAINER_ID=$(docker ps -aqf "name=rippled-service") | ||
| echo "docker-container-id=$CONTAINER_ID" >> $GITHUB_OUTPUT | ||
|
|
||
| - name: Use Node.js ${{ matrix.node-version }} | ||
| uses: actions/setup-node@v4 | ||
|
|
@@ -154,6 +157,17 @@ jobs: | |
|
|
||
| - run: npm run build | ||
|
|
||
| - name: Check if Docker container is running | ||
| id: check-docker-container | ||
| run: | | ||
| if ! docker ps | grep -q rippled-service; then | ||
| echo "INFO: Currently running docker containers:" | ||
| docker ps | ||
| echo "ERROR: rippled-service Docker container is not running" | ||
| exit 1 | ||
| fi | ||
| docker inspect ${{ steps.run-docker.outputs.docker-container-id }} | ||
|
|
||
|
Comment on lines
+160
to
+170
Contributor
There was a problem hiding this comment. 🧩 Analysis chainWait for rippled to be healthy, not merely “running”. The grep on Apply this diff to replace the step with a health-aware wait + diagnostics: - - name: Check if Docker container is running
- run: |
- docker ps
- docker ps -a
- docker inspect rippled-service
- if ! docker ps | grep -q rippled-service; then
- echo "rippled-service Docker container is not running"
- exit 1
- fi
+ - name: Wait for rippled-service to be healthy
+ run: |
+ set -Eeuo pipefail
+ echo "Checking rippled-service container existence..."
+ if ! docker ps --filter 'name=^/rippled-service$' --format '{{.Names}}' | grep -qx rippled-service; then
+ echo "rippled-service container not found in running containers."
+ docker ps -a || true
+ exit 1
+ fi
+ echo "Waiting for rippled-service (health=healthy)..."
+ for i in {1..40}; do
+ status="$(docker inspect -f '{{.State.Health.Status}}' rippled-service || echo unknown)"
+ echo "Attempt $i: health=$status"
+ if [ "$status" = "healthy" ]; then
+ echo "rippled-service is healthy."
+ break
+ fi
+ sleep 3
+ done
+ if [ "${status:-unknown}" != "healthy" ]; then
+ echo "::error::rippled-service failed to become healthy in time."
+ echo "=== docker ps ==="
+ docker ps -a || true
+ echo "=== inspect (Health) ==="
+ docker inspect -f '{{json .State.Health}}' rippled-service || true
+ echo "=== last 200 logs ==="
+ docker logs --tail 200 rippled-service || true
+ exit 1
+ fiOptional follow-up outside this hunk: make container teardown non-fatal to avoid masking earlier failures. # change step "Stop docker container"
run: docker stop rippled-service || trueTo verify, re-run the workflow; the step should show “health=healthy” before tests start. If it times out, the emitted logs will pinpoint why rippled isn’t ready. 🏁 Script executed: #!/bin/bash
set -euo pipefail
echo "PWD: $(pwd)"
echo "Searching for workflow file and snippet..."
rg -n --hidden --glob '!node_modules' 'Check if Docker container is running' || true
echo "Check for .github/workflows/nodejs.yml existence..."
if [ -f .github/workflows/nodejs.yml ]; then
echo ".github/workflows/nodejs.yml found; printing lines 140-180:"
nl -ba .github/workflows/nodejs.yml | sed -n '140,180p'
else
echo ".github/workflows/nodejs.yml not found; searching for nodejs.yml and docker inspect usage..."
rg -n --hidden --glob '!node_modules' 'nodejs.yml|docker inspect|rippled-service' || true
fiLength of output: 368 🏁 Script executed: #!/bin/bash
set -euo pipefail
FILE='.github/workflows/nodejs.yml'
echo "PWD: $(pwd)"
if [ -f "$FILE" ]; then
echo "$FILE exists; size: $(stat -c%s "$FILE" || true) bytes"
echo "---- lines 150-170 ----"
sed -n '150,170p' "$FILE" || true
echo "---- lines 156-166 ----"
sed -n '156,166p' "$FILE" || true
echo "---- search for keywords in file ----"
if command -v rg >/dev/null 2>&1; then
rg -n 'rippled-service|Check if Docker container is running|Wait for rippled-service' "$FILE" || true
else
grep -nE 'rippled-service|Check if Docker container is running|Wait for rippled-service' "$FILE" || true
fi
else
echo "$FILE not found; searching repo for matches"
if command -v rg >/dev/null 2>&1; then
rg -n 'rippled-service|docker inspect|Check if Docker container is running' || true
else
grep -RIn --exclude-dir=node_modules -E 'rippled-service|docker inspect|Check if Docker container is running' . || true
fi
fiLength of output: 2336 Wait for rippled-service to be healthy, not just running The current step only checks container presence; it can pass while health is "starting"/"unhealthy", causing integration tests to fail—poll .State.Health and surface logs on timeout. Location: .github/workflows/nodejs.yml — replace the step at lines 156–166 (duplicate at 221–227). - - name: Check if Docker container is running
- run: |
- docker ps
- docker ps -a
- docker inspect rippled-service
- if ! docker ps | grep -q rippled-service; then
- echo "rippled-service Docker container is not running"
- exit 1
- fi
+ - name: Wait for rippled-service to be healthy
+ run: |
+ set -Eeuo pipefail
+ echo "Checking rippled-service container existence..."
+ if ! docker ps --filter 'name=^/rippled-service$' --format '{{.Names}}' | grep -qx rippled-service; then
+ echo "rippled-service container not found in running containers."
+ docker ps -a || true
+ exit 1
+ fi
+ echo "Waiting for rippled-service (health=healthy)..."
+ for i in {1..40}; do
+ status="$(docker inspect -f '{{.State.Health.Status}}' rippled-service || echo unknown)"
+ echo "Attempt $i: health=$status"
+ if [ "$status" = "healthy" ]; then
+ echo "rippled-service is healthy."
+ break
+ fi
+ sleep 3
+ done
+ if [ "${status:-unknown}" != "healthy" ]; then
+ echo "::error::rippled-service failed to become healthy in time."
+ echo "=== docker ps ==="
+ docker ps -a || true
+ echo "=== inspect (Health) ==="
+ docker inspect -f '{{json .State.Health}}' rippled-service || true
+ echo "=== last 200 logs ==="
+ docker logs --tail 200 rippled-service || true
+ exit 1
+ fiOptional operational change: make teardown non-fatal to avoid masking earlier failures — change any
Collaborator
Author
There was a problem hiding this comment. It is not useful to poll for the existence of the rippled docker container. If the docker container failed to start-up correctly, there is no chance for it to restart in a healthy manner.
Contributor
There was a problem hiding this comment. @ckeshava You're absolutely right that polling won't help a failed container restart. I think there was a misunderstanding in my suggestion - I wasn't proposing to restart failed containers, but rather to wait for an already-started container to reach "healthy" status before running tests. However, since you've identified that as unnecessary, let me suggest a simpler diagnostic approach that might help identify why integration tests aren't connecting: - - name: Check if Docker container is running
- run: |
- docker ps
- docker ps -a
- docker inspect rippled-service
- if ! docker ps | grep -q rippled-service; then
- echo "rippled-service Docker container is not running"
- exit 1
- fi
+ - name: Check rippled-service connectivity
+ run: |
+ docker ps
+ if ! docker ps | grep -q rippled-service; then
+ echo "rippled-service container not found"
+ docker ps -a
+ exit 1
+ fi
+ echo "Testing rippled connectivity on port 6006..."
+ curl -f http://localhost:6006 || {
+ echo "Failed to connect to rippled on port 6006"
+ docker logs --tail 50 rippled-service
+ exit 1
+ }This approach simply verifies the service is actually accessible on the expected port and shows logs if it's not, which should help diagnose the connection issues without unnecessary waiting. ✏️ Learnings added
Collaborator
Author
There was a problem hiding this comment. Why do we need to test the
Contributor
There was a problem hiding this comment. @ckeshava You're absolutely right - if The If Your current approach of simply verifying container presence with 🧠 Learnings used |
||
| - name: Run integration test | ||
| run: npm run test:integration | ||
|
|
||
|
|
@@ -181,8 +195,11 @@ jobs: | |
| node-version: ${{ matrix.node-version }} | ||
|
|
||
| - name: Run docker in background | ||
| id: run-docker | ||
| run: | | ||
| docker run --detach --rm -p 6006:6006 --volume "${{ github.workspace }}/.ci-config/":"/etc/opt/ripple/" --name rippled-service --health-cmd="rippled server_info || exit 1" --health-interval=5s --health-retries=10 --health-timeout=2s --env GITHUB_ACTIONS=true --env CI=true --entrypoint bash ${{ env.RIPPLED_DOCKER_IMAGE }} -c "rippled -a" | ||
| CONTAINER_ID=$(docker ps -aqf "name=rippled-service") | ||
| echo "docker-container-id=$CONTAINER_ID" >> $GITHUB_OUTPUT | ||
|
|
||
| - name: Setup npm version 10 | ||
| run: | | ||
|
|
@@ -208,6 +225,17 @@ jobs: | |
|
|
||
| - run: npm run build | ||
|
|
||
| - name: Check if Docker container is running | ||
| id: check-docker-container | ||
| run: | | ||
| if ! docker ps | grep -q rippled-service; then | ||
| echo "INFO: Currently running docker containers:" | ||
| docker ps | ||
| echo "ERROR: rippled-service Docker container is not running" | ||
| exit 1 | ||
| fi | ||
| docker inspect ${{ steps.run-docker.outputs.docker-container-id }} | ||
|
|
||
| - name: Run integration test | ||
| run: npm run test:browser | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,7 @@ import type { TransactionMetadataBase } from './metadata' | |
|
|
||
| // 2^63 - 1 | ||
| const MAX_AMT = '9223372036854775807' | ||
| export const MAX_TRANSFER_FEE = 50000 | ||
|
|
||
| /** | ||
| * Transaction Flags for an MPTokenIssuanceCreate Transaction. | ||
|
|
@@ -53,8 +53,54 @@ export enum MPTokenIssuanceCreateFlags { | |
| * to clawback value from individual holders. | ||
| */ | ||
| tfMPTCanClawback = 0x00000040, | ||
|
|
||
| /** | ||
| * If set, Indicates flag lsfMPTCanLock can be changed. | ||
| */ | ||
| tmfMPTCanMutateCanLock = 0x00000002, | ||
kuan121 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| /** | ||
| * If set, Indicates flag lsfMPTRequireAuth can be changed | ||
| */ | ||
| tmfMPTCanMutateRequireAuth = 0x00000004, | ||
| /** | ||
| * If set, Indicates flag lsfMPTCanEscrow can be changed. | ||
| */ | ||
| tmfMPTCanMutateCanEscrow = 0x00000008, | ||
| /** | ||
| * If set, Indicates flag lsfMPTCanTrade can be changed. | ||
| */ | ||
| tmfMPTCanMutateCanTrade = 0x00000010, | ||
| /** | ||
| * If set, Indicates flag lsfMPTCanTransfer can be changed. | ||
| */ | ||
| tmfMPTCanMutateCanTransfer = 0x00000020, | ||
| /** | ||
| * If set, Indicates flag lsfMPTCanClawback can be changed. | ||
| */ | ||
| tmfMPTCanMutateCanClawback = 0x00000040, | ||
| /** | ||
| * If set, Allows field MPTokenMetadata to be modified. | ||
| */ | ||
| tmfMPTCanMutateMetadata = 0x00010000, | ||
| /** | ||
| * If set, Allows field TransferFee to be modified. | ||
| */ | ||
| tmfMPTCanMutateTransferFee = 0x00020000, | ||
| } | ||
|
|
||
| /* eslint-disable no-bitwise -- Need bitwise operations to replicate rippled behavior */ | ||
| export const tmfMPTokenIssuanceCreateMutableMask = ~( | ||
| MPTokenIssuanceCreateFlags.tmfMPTCanMutateCanLock | | ||
| MPTokenIssuanceCreateFlags.tmfMPTCanMutateRequireAuth | | ||
| MPTokenIssuanceCreateFlags.tmfMPTCanMutateCanEscrow | | ||
| MPTokenIssuanceCreateFlags.tmfMPTCanMutateCanTrade | | ||
| MPTokenIssuanceCreateFlags.tmfMPTCanMutateCanTransfer | | ||
| MPTokenIssuanceCreateFlags.tmfMPTCanMutateCanClawback | | ||
| MPTokenIssuanceCreateFlags.tmfMPTCanMutateMetadata | | ||
| MPTokenIssuanceCreateFlags.tmfMPTCanMutateTransferFee | ||
| ) | ||
| /* eslint-enable no-bitwise */ | ||
|
|
||
| /** | ||
| * Map of flags to boolean values representing {@link MPTokenIssuanceCreate} transaction | ||
| * flags. | ||
|
|
@@ -63,12 +109,68 @@ export enum MPTokenIssuanceCreateFlags { | |
| */ | ||
| export interface MPTokenIssuanceCreateFlagsInterface | ||
| extends GlobalFlagsInterface { | ||
| /** | ||
| * If set, indicates that the MPT can be locked both individually and globally. | ||
| * If not set, the MPT cannot be locked in any way. | ||
| */ | ||
| tfMPTCanLock?: boolean | ||
| /** | ||
| * If set, indicates that individual holders must be authorized. | ||
| * This enables issuers to limit who can hold their assets. | ||
| */ | ||
| tfMPTRequireAuth?: boolean | ||
| /** | ||
| * If set, indicates that individual holders can place their balances into an escrow. | ||
| */ | ||
| tfMPTCanEscrow?: boolean | ||
| /** | ||
| * If set, indicates that individual holders can trade their balances | ||
| * using the XRP Ledger DEX or AMM. | ||
| */ | ||
| tfMPTCanTrade?: boolean | ||
| /** | ||
| * If set, indicates that tokens may be transferred to other accounts | ||
| * that are not the issuer. | ||
| */ | ||
| tfMPTCanTransfer?: boolean | ||
| /** | ||
| * If set, indicates that the issuer may use the Clawback transaction | ||
| * to clawback value from individual holders. | ||
| */ | ||
| tfMPTCanClawback?: boolean | ||
|
|
||
| /** | ||
| * If set, Indicates flag lsfMPTCanLock can be changed. | ||
| */ | ||
| tmfMPTCanMutateCanLock?: boolean | ||
|
Collaborator
There was a problem hiding this comment. These flags are only available for MutableFlags field. Should it be separated into its own interface, similar to what we did with
Collaborator
Author
There was a problem hiding this comment.
Collaborator
There was a problem hiding this comment.
Is the xrpl.org documentation updated already?
Collaborator
Author
There was a problem hiding this comment. No, the XLS specification is a good source of info for unreleased amendments. Usually, you can find the pre-release documentation for amendments on https://opensource.ripple.com/ as well, however LendingProtocol docs have not been published yet. You can look at pre-release docs for other amendments on that website.
Collaborator
There was a problem hiding this comment. We were separating the flags that can be set with
Collaborator
Author
|
||
| /** | ||
| * If set, Indicates flag lsfMPTRequireAuth can be changed. | ||
| */ | ||
| tmfMPTCanMutateRequireAuth?: boolean | ||
| /** | ||
| * If set, Indicates flag lsfMPTCanEscrow can be changed. | ||
| */ | ||
| tmfMPTCanMutateCanEscrow?: boolean | ||
| /** | ||
| * If set, Indicates flag lsfMPTCanTrade can be changed. | ||
| */ | ||
| tmfMPTCanMutateCanTrade?: boolean | ||
| /** | ||
| * If set, Indicates flag lsfMPTCanTransfer can be changed. | ||
| */ | ||
| tmfMPTCanMutateCanTransfer?: boolean | ||
| /** | ||
| * If set, Indicates flag lsfMPTCanClawback can be changed. | ||
| */ | ||
| tmfMPTCanMutateCanClawback?: boolean | ||
| /** | ||
| * If set, Allows field MPTokenMetadata to be modified. | ||
| */ | ||
| tmfMPTCanMutateMetadata?: boolean | ||
| /** | ||
| * If set, Allows field TransferFee to be modified. | ||
| */ | ||
| tmfMPTCanMutateTransferFee?: boolean | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -120,13 +222,14 @@ export interface MPTokenIssuanceCreate extends BaseTransaction { | |
| MPTokenMetadata?: string | ||
|
|
||
| Flags?: number | MPTokenIssuanceCreateFlagsInterface | ||
|
Collaborator
There was a problem hiding this comment. Flags cannot accept the mutable flags so the above change will be needed |
||
| MutableFlags?: number | ||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
Collaborator
There was a problem hiding this comment. Can you add docstring for this?
Collaborator
Author
There was a problem hiding this comment. I added a recent commit to fix this. However, I could not find the documentation for the
Collaborator
Author
There was a problem hiding this comment. Update: I have added a docstring for the |
||
| } | ||
|
|
||
| export interface MPTokenIssuanceCreateMetadata extends TransactionMetadataBase { | ||
| mpt_issuance_id?: string | ||
| } | ||
|
|
||
| /* eslint-disable max-lines-per-function, max-statements -- Not needed to reduce function */ | ||
| /** | ||
| * Verify the form and type of an MPTokenIssuanceCreate at runtime. | ||
| * | ||
|
|
@@ -141,6 +244,17 @@ export function validateMPTokenIssuanceCreate( | |
| validateOptionalField(tx, 'MPTokenMetadata', isString) | ||
| validateOptionalField(tx, 'TransferFee', isNumber) | ||
| validateOptionalField(tx, 'AssetScale', isNumber) | ||
| validateOptionalField(tx, 'MutableFlags', isNumber) | ||
kuan121 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| if ( | ||
| tx.MutableFlags != null && | ||
| // eslint-disable-next-line no-bitwise -- Need bitwise operations to replicate rippled behavior | ||
| tx.MutableFlags & tmfMPTokenIssuanceCreateMutableMask | ||
| ) { | ||
| throw new ValidationError( | ||
| 'MPTokenIssuanceCreate: Invalid MutableFlags value', | ||
| ) | ||
| } | ||
|
|
||
| if ( | ||
| typeof tx.MPTokenMetadata === 'string' && | ||
|
|
@@ -202,4 +316,4 @@ export function validateMPTokenIssuanceCreate( | |
| } | ||
| } | ||
| } | ||
| /* eslint-enable max-lines-per-function, max-statements */ | ||
Uh oh!
There was an error while loading. Please reload this page.