Skip to content

deposit wallet init#21

Open
arun-poly wants to merge 2 commits intomainfrom
deposit-wallet
Open

deposit wallet init#21
arun-poly wants to merge 2 commits intomainfrom
deposit-wallet

Conversation

@arun-poly
Copy link
Copy Markdown

@arun-poly arun-poly commented Apr 9, 2026

Note

Medium Risk
Introduces new EIP-712 v2 order types and signing flow, including special-casing signer validation for POLY_DEPOSIT_WALLET, which can affect correctness of order signing/hashing. Risk is moderate because it touches cryptographic signing inputs but is additive and doesn’t modify the existing v1 builder.

Overview
Adds a new v2 order-building API via ExchangeOrderBuilderV2, including OrderV2/SignedOrderV2 models, v2 EIP-712 struct/constants, and exports from index.ts.

The v2 typed-data payload updates the signed fields (adds timestamp/metadata/builder, and keeps expiration as an API-only field not included in the EIP-712 message), and extends SignatureType with POLY_DEPOSIT_WALLET while relaxing the “signer must match signer.getAddress()” check for that signature type.

Reviewed by Cursor Bugbot for commit f5c8bcc. Bugbot is set up for automated code reviews on this repo. Configure here.

@arun-poly arun-poly requested a review from a team as a code owner April 9, 2026 10:57
Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit f5c8bcc. Configure here.

takerAmount,
side,
signatureType: resolvedSignatureType,
timestamp: timestamp ?? Date.now().toString(),
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Default timestamp uses milliseconds instead of seconds

High Severity

The default timestamp value uses Date.now().toString(), which returns milliseconds since epoch. In blockchain contexts, timestamps are in seconds (block.timestamp is unix seconds). The sibling expiration field is explicitly documented as "unix seconds, '0' = no expiration", strongly indicating timestamp is also expected in seconds. A millisecond value will be ~1000x too large, causing incorrect order timestamps and likely on-chain validation failures.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f5c8bcc. Configure here.

@@ -0,0 +1,21 @@
// V2 Exchange constants
// Domain name is shared with V1; only the version changes.
export const PROTOCOL_NAME_V2 = 'Polymarket CTF Exchange';
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Exported PROTOCOL_NAME_V2 constant is never used

Low Severity

PROTOCOL_NAME_V2 is exported but never imported or referenced anywhere in the codebase. The V2 builder imports PROTOCOL_NAME from the V1 constants file instead. The comment notes the domain name is shared with V1, making this constant redundant with the identically-valued PROTOCOL_NAME. Having both creates a maintenance risk where they could diverge unintentionally.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit f5c8bcc. Configure here.

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