Skip to content

release v1.1.0 - #83

Open
roger-gan wants to merge 7 commits into
mainfrom
sync/upstream-typescript-2026-07-30
Open

release v1.1.0#83
roger-gan wants to merge 7 commits into
mainfrom
sync/upstream-typescript-2026-07-30

Conversation

@roger-gan

Copy link
Copy Markdown
Contributor

Description

Tests

Checklist

  • I have formatted and linted my code
  • All new and existing tests pass
  • My commits are signed (required for merge) -- you may need to rebase if you initially pushed unsigned commits

@Will-Guan Will-Guan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Finding

  • Medium — Parsed token symbols can be silently ignored.
    The TRON batch, exact, and upto price-parsing paths can discard the symbol returned by parseMoney(), causing prices such as "1 USDD" or "$1 USDD" to fall back to the network default USDT asset. Preserve the symbol during token resolution or explicitly reject unsupported suffixes.

Suggestion

  • Enforce a consistent zero-amount policy.
    Core and EVM intentionally truncate sub-atomic prices to "0", while TRON exact and upto reject them locally. If paid routes must charge at least one atomic unit, validate BigInt(parsedPrice.amount) > 0n centrally when building PaymentRequirements, rather than adding a TRON-batch-only guard.

@@ -240,7 +243,7 @@ export class BatchSettlementTronScheme implements SchemeNetworkServer {
return { amount: price.amount, asset: price.asset, extra: price.extra || {} };
}

const amount = this.parseMoneyToDecimal(price);
const amount = parseMoney(price).amount;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Medium · bug — Preserve or reject the parsed token symbol

parseMoney() returns { amount, symbol }, but this line keeps only amount, so the fallback conversion always uses the network default asset. For example, parsePrice("1 USDD", Nile) is silently converted to 1_000_000 units of Nile USDT instead of USDD. Unsupported symbols can likewise be treated as USDT without an error.

The EVM implementation preserves symbol and passes it to asset resolution. Please either propagate the symbol into the TRON token lookup/conversion path or explicitly reject non-USD suffixes that this path does not support.

@@ -76,7 +79,7 @@ export class ExactTronScheme implements SchemeNetworkServer {
}

// Parse Money to decimal number
const amount = this.parseMoneyToDecimal(price);
const amount = parseMoney(price).amount;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same finding — the token symbol is lost on the dollar-prefixed path

A value such as "$1 USDD" does not match the token-price branch above, because that expression does not accept the $ prefix. It therefore reaches parseMoney(), where USDD is parsed successfully but discarded by selecting only .amount, and the price falls back to the default USDT asset.

Please preserve and resolve the returned symbol, or explicitly reject this syntax instead of silently changing the requested asset.

@@ -61,7 +63,7 @@ export class UptoTronScheme implements SchemeNetworkServer {
return parseTokenPrice(price.trim(), network);
}

const amount = this.parseMoneyToDecimal(price);
const amount = parseMoney(price).amount;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same finding — the token symbol is lost on the dollar-prefixed path

As in the exact implementation, "$1 USDD" bypasses the token-price branch, after which parseMoney() extracts USDD but this line discards it. The resulting requirement is then denominated in the default USDT asset.

Please handle the parsed symbol consistently with the token-price path, or reject unsupported suffixes explicitly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants