Skip to content

fix: exclude marketplace fees from private listings - #1714

Merged
ryanio merged 10 commits into
mainfrom
devin/1752536139-fix-marketplace-fees-private-listings
Jul 15, 2025
Merged

fix: exclude marketplace fees from private listings#1714
ryanio merged 10 commits into
mainfrom
devin/1752536139-fix-marketplace-fees-private-listings

Conversation

@ryanio

@ryanio ryanio commented Jul 15, 2025

Copy link
Copy Markdown
Collaborator

Fix marketplace fees in private listings and prevent CI order conflicts

Summary

This PR addresses two issues:

  1. Primary fix: Marketplace fees are now properly excluded from private listings (when buyerAddress is specified in createListing). The getFees() method in src/sdk.ts has been modified to filter out marketplace fees for private listings using the OPENSEA_FEE_RECIPIENT constant.

  2. CI improvement: Added a getRandomExpiration() helper function that generates random expiration times (15-60 minutes) to prevent "Order already exists" validation errors in integration tests.

  3. Security fix: Replaced biased modulo operation with rejection sampling in random number generation to resolve CodeQL security alerts about cryptographically secure random numbers.

Key changes:

  • Modified getFees() method to detect private listings via isPrivateListing parameter and exclude marketplace fees
  • Added OPENSEA_FEE_RECIPIENT constant to src/constants.ts
  • Created getRandomExpiration() helper in test/integration/setup.ts using secure rejection sampling
  • Updated all integration tests to use random expiration times
  • Added comprehensive integration tests for private listings

Review & Testing Checklist for Human

  • Verify marketplace fee exclusion works end-to-end: Create a private listing via the API and confirm marketplace fees are not included in the order's consideration array
  • Test regular listings still include fees: Create a regular listing and verify marketplace fees are still present
  • Validate OPENSEA_FEE_RECIPIENT constant: Confirm 0x0000a26b00c1f0df003000390027140000faa719 is the correct marketplace fee recipient address
  • Regression testing: Verify that fee calculations for different payment tokens and collection fees still work correctly
  • Monitor CI for order conflicts: Check that integration tests no longer fail with "Order already exists" errors

Recommended test plan: Create both private and regular listings using the SDK with the same NFT to verify the fee behavior difference.


Diagram

%%{ init : { "theme" : "default" }}%%
graph TD
    SDK["src/sdk.ts<br/>getFees() method"]:::major-edit
    Constants["src/constants.ts<br/>OPENSEA_FEE_RECIPIENT"]:::minor-edit
    Setup["test/integration/setup.ts<br/>getRandomExpiration()"]:::minor-edit
    
    PostOrder["test/integration/postOrder.spec.ts<br/>Order posting tests"]:::minor-edit
    PrivateListing["test/integration/privateListing.spec.ts<br/>Private listing tests"]:::minor-edit
    
    API["OpenSea API<br/>Order validation"]:::context
    
    SDK -->|"excludes marketplace fees<br/>for private listings"| API
    Constants -->|"identifies marketplace<br/>fee recipient"| SDK
    Setup -->|"prevents order conflicts"| PostOrder
    Setup -->|"prevents order conflicts"| PrivateListing
    
    subgraph Legend
        L1[Major Edit]:::major-edit
        L2[Minor Edit]:::minor-edit
        L3[Context/No Edit]:::context
    end
    
    classDef major-edit fill:#90EE90
    classDef minor-edit fill:#87CEEB
    classDef context fill:#FFFFFF
Loading

Notes

  • The core fix addresses GitHub issue Error: Server Error: Validation error: Private transactions should not include marketplace fees. #1710 about marketplace fees in private transactions
  • Local integration tests pass (22 passing, 0 failing) with the new random expiration logic
  • The isPrivateListing parameter in getFees() defaults to false to maintain backward compatibility
  • Random expiration times range from 15-60 minutes from current time to provide sufficient variability while keeping reasonable test execution times
  • Security fix uses rejection sampling to eliminate modulo bias in cryptographically secure random number generation
  • All CI checks now pass including CodeQL security analysis

Session info: Requested by @ryanio
Link to Devin run: https://app.devin.ai/sessions/ab90a4a425d545c2ad20b28fb238165e

- Modified getFees method to accept isPrivateListing parameter
- Added marketplace fee filtering logic for private listings
- Added integration tests for private listing fee behavior
- Fixes issue #1710 where private listings were rejected by API

The fix ensures that when buyerAddress is specified in createListing,
marketplace fees (2.5% non-required fees) are excluded from the
consideration items, preventing API rejection of private transactions.

Co-Authored-By: Ryan Ghods <ralxzryan@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor

Original prompt from Ryan:

 In opensea-js, please fix this issue, do not marketplace fees in private transactions  (when buyerAddress is specified in createListing). add a test for it that the api returns properly
https://github.com/ProjectOpenSea/opensea-js/issues/1710

@devin-ai-integration

Copy link
Copy Markdown
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

devin-ai-integration Bot and others added 3 commits July 15, 2025 00:09
…ering

- Remove isPrivateListing parameter, detect from buyerAddress presence
- Use OPENSEA_FEE_RECIPIENT constant for marketplace fee identification
- Simplify isNotMarketplaceFee logic to use recipient address comparison
- Update all getFees calls to use buyerAddress parameter

Based on user feedback to simplify the implementation.

Co-Authored-By: Ryan Ghods <ralxzryan@gmail.com>
- Add 'as string' to TOKEN_ADDRESS_MAINNET and TOKEN_ID_MAINNET definitions
- Remove 'as string' from all usage sites for cleaner type casting

Based on user feedback to improve TypeScript typing consistency.

Co-Authored-By: Ryan Ghods <ralxzryan@gmail.com>
- Rename buyerAddress parameter in getFees to isPrivateListing with default false
- Move OPENSEA_FEE_RECIPIENT constant to src/constants.ts for better organization
- Move private listing test to integration folder to fix CI unit test failure
- Update getFees calls to use new isPrivateListing parameter

Co-Authored-By: Ryan Ghods <ralxzryan@gmail.com>
Comment thread test/integration/privateListing.spec.ts Outdated

expect(order.protocolData.parameters.consideration).to.exist;

const marketplaceFeeRecipient =

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Use the OPENSEA_FEE_RECIPIENT const

devin-ai-integration Bot and others added 2 commits July 15, 2025 00:25
- Import OPENSEA_FEE_RECIPIENT from src/constants.ts
- Replace hardcoded marketplace fee recipient address with constant
- Addresses GitHub comment feedback for better code consistency

Co-Authored-By: Ryan Ghods <ralxzryan@gmail.com>
- Add getRandomExpiration() helper that returns 15-60 minutes from now
- Update all order posting integration tests to use random expiration times
- Update private listing tests for consistency
- Prevents 'Order already exists' validation errors in CI by adding variability

Co-Authored-By: Ryan Ghods <ralxzryan@gmail.com>
Comment thread test/utils/utils.ts Outdated
}
};

export const getRandomExpiration = (): number => {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Move this to setup.ts

devin-ai-integration Bot and others added 2 commits July 15, 2025 00:43
…mExpiration

- Use crypto.randomBytes() for cryptographically secure random number generation
- Fixes CodeQL security alerts about insecure random number usage
- Maintains same API and functionality for test utilities

Co-Authored-By: Ryan Ghods <ralxzryan@gmail.com>
- Move getRandomExpiration function from test/utils/utils.ts to test/integration/setup.ts
- Update imports in postOrder.spec.ts and privateListing.spec.ts
- Addresses GitHub comment from @ryanio to consolidate test utilities in setup.ts

Co-Authored-By: Ryan Ghods <ralxzryan@gmail.com>
Comment thread test/integration/setup.ts Fixed
devin-ai-integration Bot and others added 2 commits July 15, 2025 00:47
- Use import { randomBytes } from 'crypto' instead of require()
- Follows ES6 import patterns and resolves CodeQL security alerts
- Maintains same cryptographically secure random number generation

Co-Authored-By: Ryan Ghods <ralxzryan@gmail.com>
- Replace modulo operation with rejection sampling in getRandomExpiration()
- Eliminates CodeQL security alert about biased cryptographic random numbers
- Maintains same functionality while ensuring unbiased distribution

Co-Authored-By: Ryan Ghods <ralxzryan@gmail.com>
@ryanio
ryanio merged commit 49e71fa into main Jul 15, 2025
7 checks passed
@ryanio
ryanio deleted the devin/1752536139-fix-marketplace-fees-private-listings branch July 15, 2025 00:55
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