Skip to content

fix: @meshsdk/contract -> MeshMarketplaceContract -> purchaseAsset-> TypeError: Cannot mix BigInt and other types, use explicit conversions #714

Open
MoFayaz wants to merge 1 commit into
MeshJS:mainfrom
MoFayaz:main

Conversation

@MoFayaz

@MoFayaz MoFayaz commented Sep 6, 2025

Copy link
Copy Markdown
Contributor

Thank you for contributing to Mesh! We appreciate your effort and dedication to improving this project. To ensure that your contribution is in line with the project's guidelines and can be reviewed efficiently, please fill out the template below.

Remember to follow our Contributing Guide before submitting your pull request.

Summary

Please provide a brief, concise summary of the changes in your pull request. Explain the problem you are trying to solve and the solution you have implemented.

@meshsdk/contract -> MeshMarketplaceContract -> purchaseAsset

I got this error when calling the purchaseAsset function. fixed when converting to number

TypeError: Cannot mix BigInt and other types, use explicit conversions

in compiled code https://unpkg.com/@meshsdk/contract@1.9.0-beta.73/dist/index.js,
let ownerToReceiveLovelace = inputDatum.fields[1].int * this.feePercentageBasisPoint / 1e4;

Changed to
let ownerToReceiveLovelace = Number(inputDatum.fields[1].int) * this.feePercentageBasisPoint / 1e4;

Affect components

Please indicate which part of the Mesh Repo

  • @meshsdk/common
  • @meshsdk/contract
  • @meshsdk/core
  • @meshsdk/core-csl
  • @meshsdk/core-cst
  • @meshsdk/hydra
  • @meshsdk/provider
  • @meshsdk/react
  • @meshsdk/svelte
  • @meshsdk/transaction
  • @meshsdk/wallet
  • Mesh playground (i.e. https://meshjs.dev/)
  • Mesh CLI

Type of Change

Please mark the relevant option(s) for your pull request:

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • [x ] Bug fix (non-breaking change which fixes an issue)
  • Code refactoring (improving code quality without changing its behavior)
  • Documentation update (adding or updating documentation related to the project)

Related Issues

Please add the related issue here if any

Checklist

Please ensure that your pull request meets the following criteria:

  • My code is appropriately commented and includes relevant documentation, if necessary
  • I have added tests to cover my changes, if necessary
  • I have updated the documentation, if necessary
  • All new and existing tests pass (i.e. npm run test)
  • The build is pass (i.e. npm run build)

Additional Information

If you have any additional information or context to provide, such as screenshots, relevant issues, or other details, please include them here.

@MoFayaz MoFayaz changed the title Update offchain.ts @meshsdk/contract -> MeshMarketplaceContract -> purchaseAsset-> TypeError: Cannot mix BigInt and other types, use explicit conversions Sep 6, 2025
@MoFayaz MoFayaz changed the title @meshsdk/contract -> MeshMarketplaceContract -> purchaseAsset-> TypeError: Cannot mix BigInt and other types, use explicit conversions fix: @meshsdk/contract -> MeshMarketplaceContract -> purchaseAsset-> TypeError: Cannot mix BigInt and other types, use explicit conversions Sep 6, 2025
@OscarOzaine

Copy link
Copy Markdown

Code Review — PR #714

The fix correctly replaces the as number compile-time lies with Number() calls that actually convert BigInt at runtime, which resolves the TypeError crash. However, the fix is incomplete and introduces/exposes several issues in the same function.


🔴 Critical

Shared this.mesh builder — TOCTOU on concurrent calls (offchain.ts:161)

this.mesh is a single MeshTxBuilder instance shared across all method calls. const tx = this.mesh... is just an alias, not a copy. If two purchaseAsset calls execute concurrently on the same contract instance (common in server-side dApps), the second complete() overwrites this.mesh.txHex before the first caller reads it — caller A silently receives caller B's transaction hex, signs it, and submits a transaction spending the wrong UTxO.


🟡 Medium — Introduced by this PR

Number(bigint) precision loss — wrong fee and seller amounts for prices > ~9M ADA

// line 182 — ownerToReceiveLovelace
(Number(inputDatum.fields[1].int) * this.feePercentageBasisPoint) / 10000

// line 199 — sellerToReceiveLovelace
Number(inputDatum.fields[1].int) + Number(inputLovelace)

Number.MAX_SAFE_INTEGER = 9,007,199,254,740,991 lovelace ≈ 9,007,199 ADA. For prices above this threshold, Number(bigint) silently rounds the value to the nearest representable float64, producing wrong fee and seller payment amounts that may cause the on-chain validator to reject the transaction.

Additionally, at the MAX_SAFE_INTEGER boundary the float addition is off-by-1: 9007199254740991 + 2000000 in JS float arithmetic = 9007199256740992, but the correct BigInt result is 9007199256740991.

Suggested fix — keep arithmetic in BigInt throughout:

const priceBig = BigInt(inputDatum.fields[1].int);
const feeBig = BigInt(Math.round(this.feePercentageBasisPoint));

// ceiling division in BigInt (equivalent to Math.ceil)
let ownerToReceiveLovelace = (priceBig * feeBig + 9999n) / 10000n;
if (this.feePercentageBasisPoint > 0 && ownerToReceiveLovelace < 1000000n) {
  ownerToReceiveLovelace = 1000000n;
}
// ...
const sellerToReceiveLovelace = priceBig + BigInt(inputLovelace);

// Only convert to string at the txOut quantity boundary
quantity: ownerToReceiveLovelace.toString()
quantity: sellerToReceiveLovelace.toString()

🟡 Medium — Pre-existing in same function

Free listing (price = 0) forces buyer to pay spurious 1-ADA fee (line 184)

When price = 0 and feePercentageBasisPoint > 0: computed fee = 0, which is < 1_000_000, so the floor kicks in and the buyer is charged 1 ADA to the marketplace owner. The on-chain Aiken validator computes datum.price * fee / 10000 = 0 and only requires value_geq(0) — there is no on-chain floor. The 1-ADA floor is purely an off-chain tax with no contract backing.

1-ADA minimum floor has no on-chain enforcement — any buyer who builds the transaction directly (bypassing this SDK) can pay only the on-chain required amount (e.g., 5 lovelace for a 5,000-lovelace listing at 100bps) and the validator will accept it. The marketplace owner cannot rely on the 1-ADA minimum as a guaranteed invariant.

Non-null assertion crash on missing lovelace entry (line 157)

const inputLovelace = marketplaceUtxo.output.amount.find(
  (a) => a.unit === "lovelace",
)!.quantity;  // throws TypeError if no lovelace entry

If a UTxO (from a test fixture, custom IWallet, or provider gap) has no lovelace entry, .find() returns undefined and !.quantity throws an opaque TypeError with no domain-meaningful message.

No upper-bound validation on feePercentageBasisPoint (line 55)

A value > 10000 (> 100%) is accepted silently. With feePercentageBasisPoint = 20000, ownerToReceiveLovelace = 2 * price and the total outputs require 3× the listing price from the buyer's UTxOs — tx.complete() throws or the transaction is rejected.


Summary

# Severity Issue
1 🔴 High Shared this.mesh TOCTOU — concurrent calls swap tx hexes
2 🟡 Medium Number(bigint) precision loss — wrong fee/seller amounts above ~9M ADA
3 🟡 Medium Off-by-1 at MAX_SAFE_INTEGER boundary in seller payment sum
4 🟡 Medium Free listing (price=0) triggers spurious 1-ADA fee with no on-chain backing
5 🟡 Medium 1-ADA minimum floor is not enforced on-chain — bypassable
6 🟡 Medium Non-null assertion on !.quantity crashes on UTxOs missing a lovelace entry
7 🟡 Medium No upper-bound validation on feePercentageBasisPoint

The PR should replace Number(bigint) with BigInt-native arithmetic (see suggestion above) before merging. Issues 4–7 are pre-existing but worth addressing in the same PR since you're already touching this function.

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