Skip to content

fix(contract): safe BigInt arithmetic in MeshMarketplaceContract.purchaseAsset#832

Open
OscarOzaine wants to merge 2 commits into
MeshJS:mainfrom
OscarOzaine:fix/marketplace-purchaseasset-bigint-safe
Open

fix(contract): safe BigInt arithmetic in MeshMarketplaceContract.purchaseAsset#832
OscarOzaine wants to merge 2 commits into
MeshJS:mainfrom
OscarOzaine:fix/marketplace-purchaseasset-bigint-safe

Conversation

@OscarOzaine

Copy link
Copy Markdown

Summary

This is a suggested improvement follow-up to #714 (which fixed the TypeError: Cannot mix BigInt and other types crash in purchaseAsset).

While #714's Number() conversion resolves the crash, it introduces silent precision loss for NFT prices above Number.MAX_SAFE_INTEGER (~9,007,199 ADA) and leaves several other issues in the same function unaddressed. This PR addresses them all.

Changes

purchaseAsset — BigInt-safe arithmetic (fixes #714's root cause)

// Before (PR #714): Number() conversion loses precision above ~9M ADA
let ownerToReceiveLovelace =
  (Number(inputDatum.fields[1].int) * this.feePercentageBasisPoint) / 10000;

// After: all arithmetic stays in BigInt; convert to string only at txOut boundary
const priceBig = BigInt(inputDatum.fields[1].int);
const feeBig = BigInt(Math.round(this.feePercentageBasisPoint));
let ownerToReceiveLovelace = (priceBig * feeBig + 9999n) / 10000n; // ceiling division
  • (a * b + 9999n) / 10000n is exact BigInt ceiling division — equivalent to Math.ceil without any float arithmetic.
  • Seller payment: priceBig + BigInt(lovelaceEntry.quantity) — no IEEE-754 rounding regardless of price magnitude.

Free-listing spurious fee fix

Previously, a listing with price = 0 would hit the 1-ADA minimum floor and charge the buyer 1 ADA to the marketplace owner, even though the on-chain validator requires value_geq(0) (no floor). Added priceBig > 0n guard.

lovelaceEntry — descriptive error instead of opaque crash

// Before: throws "TypeError: Cannot read properties of undefined (reading 'quantity')"
const inputLovelace = marketplaceUtxo.output.amount.find((a) => a.unit === "lovelace")!.quantity;

// After: throws a meaningful domain error
const lovelaceEntry = marketplaceUtxo.output.amount.find((a) => a.unit === "lovelace");
if (!lovelaceEntry) throw new Error("purchaseAsset: marketplace UTxO is missing a lovelace entry");

Constructor — feePercentageBasisPoint range validation

A value > 10000 (> 100%) silently produces ownerToReceiveLovelace > price, making the transaction unbalanceable. Now throws early with a clear message.

Issues addressed

# Severity Issue
1 Medium Number(bigint) precision loss for prices > ~9M ADA → wrong fee and seller amounts
2 Medium Float addition off-by-1 at MAX_SAFE_INTEGER boundary in seller payment
3 Medium Free listing (price=0) triggers spurious 1-ADA mandatory fee
4 Medium Non-null assertion crash with opaque error on UTxOs missing lovelace
5 Medium feePercentageBasisPoint > 10000 accepted silently

The shared this.mesh builder TOCTOU (concurrent calls swapping tx hexes) is a broader architectural issue across all contract methods and is left for a separate discussion.

🤖 Generated with Claude Code

OscarOzaine and others added 2 commits June 7, 2026 07:25
…input

OfflineEvaluator.evaluateTx pushed resolved UTxOs into the caller's
additionalUtxos array (a surprising side effect that accumulated across
repeated calls) and fetched each parent transaction's UTxOs sequentially,
so latency scaled linearly with the number of distinct input transactions.

It now copies the input array and resolves distinct parent transactions
concurrently via Promise.all, leaving the caller's array untouched. The
two in-tree callers pass a freshly built array and read only the return
value, so behavior is unchanged for them.

Adds a no-mutation assertion to the existing evaluator test, which already
resolves inputs across several parent transactions (exercising the parallel
path).

Refs audit findings PERF-001, REL-003

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…aseAsset

Replaces float-based lovelace arithmetic with BigInt throughout purchaseAsset
to avoid silent precision loss for NFT prices above Number.MAX_SAFE_INTEGER
(~9,007,199 ADA). The previous `as number` casts were TypeScript-only lies
that caused a TypeError at runtime; converting via Number() fixed the crash
but introduced silent rounding errors for large prices.

Changes:
- All fee and seller-payment calculations now use BigInt operators; quantities
  are only converted to string at the txOut boundary.
- Ceiling division implemented as (a * b + 9999n) / 10000n instead of
  Math.ceil(float), which is exact and matches the on-chain semantics.
- 1-ADA minimum fee floor is now guarded by priceBig > 0n so free listings
  (price = 0) no longer incur a spurious mandatory fee.
- lovelaceEntry guard replaces the !.quantity non-null assertion with a
  descriptive error instead of an opaque TypeError.
- Constructor validates feePercentageBasisPoint is in [0, 10000] and throws
  early rather than silently producing unbalanceable transactions.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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.

1 participant