Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Openocean #1610

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Openocean #1610

wants to merge 3 commits into from

Conversation

VolodymyrBg
Copy link

Motivation

The postOrder method in the OpenSea SDK lacked proper input validation, which could lead to unnecessary API calls with invalid parameters. This was marked with a TODO comment in the code:
// TODO: Validate apiOptions. Avoid API calls that will definitely fail

By implementing proper validation, we can:

  1. Prevent unnecessary API calls that would fail
  2. Provide better error messages to developers
  3. Improve the developer experience by failing fast with clear error messages

Solution

I've added comprehensive validation for the apiOptions parameter in the postOrder method. The validation includes:

  1. Required fields checking:
    • side
    • protocolAddress
    • order
  2. Protocol value validation (only 'seaport' is supported)
  3. Side value validation (must be either 'ask' or 'bid')
  4. Protocol address format validation (must be a valid Ethereum address)

The changes are accompanied by a comprehensive test suite in test/api/postOrder.validation.spec.ts that verifies all validation cases.

Each validation failure provides a clear, descriptive error message to help developers quickly identify and fix issues in their code.

Added validation for apiOptions in postOrder method to prevent API calls that would definitely fail. The validation includes:
- Required fields checking
- Protocol value validation
- Side value validation
- Protocol address format validation

Resolves TODO comment in api.ts
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