Skip to content

fix: validate amount before calling parseUnits in _getPriceParameters#1974

Open
Sertug17 wants to merge 2 commits into
ProjectOpenSea:mainfrom
Sertug17:fix/validate-amount-before-parseunits
Open

fix: validate amount before calling parseUnits in _getPriceParameters#1974
Sertug17 wants to merge 2 commits into
ProjectOpenSea:mainfrom
Sertug17:fix/validate-amount-before-parseunits

Conversation

@Sertug17

Copy link
Copy Markdown

Problem

In _getPriceParameters the null guard was placed after amount.toString() was already called:

// BEFORE (buggy)
const amountWei = parseUnits(amount.toString(), decimals) // crashes if null
if (amount == null || amountWei < 0) { throw ... }        // too late

If amount is null or undefined (possible from JavaScript callers or edge cases), this produces an unhandled TypeError: Cannot read properties of null (reading 'toString') instead of the intended validation error.

Fix

Move the null check — and the isEther/OFFER precondition — before parseUnits is called. The negative-bigint guard stays after since it requires amountWei to exist:

// AFTER (fixed)
if (amount == null) { throw new Error('Starting price must be a number >= 0') }
if (isEther && orderSide === OrderSide.OFFER) { throw ... }
const amountWei = parseUnits(amount.toString(), decimals) // safe now
if (amountWei < 0) { throw new Error('Starting price must be a number >= 0') }

Testing

Added a regression test in test/sdk/misc.spec.ts confirming that passing null as amount does not surface a raw TypeError.

Serhat Dolmacı added 2 commits May 31, 2026 21:13
Chain.Soneium and Chain.AnimeChain were present in the Chain enum but
missing from getOfferPaymentToken() and getListingPaymentToken() switch
statements, causing a runtime Error when creating listings or offers on
these chains.

Both chains are OP Stack based:
- Soneium (chain ID 1868): uses canonical OP WETH 0x4200...0006
- AnimeChain (chain ID 69000): uses canonical OP WETH 0x4200...0006

Listing payment token for both is native ETH (0x0000...0000).

Added test coverage for both chains.
The null guard and negative-amount check were placed *after*
amount.toString() was already called, meaning a null or undefined
amount would throw a TypeError inside parseUnits before the explicit
guard could fire.

Fix: move the null check (and the isEther/OFFER check, which is also
a precondition) *before* parseUnits is invoked. The negative-bigint
check stays after, since it requires amountWei to be computed first.

Adds a regression test to misc.spec.ts verifying that passing null
does not produce a raw TypeError.
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