Add tokenId support to ERC1155 signature generation and remove validityStartTimestamp #903
Conversation
WalkthroughThe changes update the ERC1155 signature generation logic by removing Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant SDKv4
participant SDKv5
Client->>Server: Request signature generation
alt Using v5 SDK
Server->>SDKv5: Generate signature (no validityStartTimestamp)
SDKv5-->>Server: signedPayload
else Using v4 SDK
alt tokenId present
Server->>SDKv4: generateFromTokenId(tokenId as BigInt)
else tokenId absent
Server->>SDKv4: generate()
end
SDKv4-->>Server: signedPayload
Server->>Server: Debug log signedPayload
end
Server-->>Client: Return signedPayload
Possibly related PRs
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/server/routes/contract/extensions/erc1155/read/signature-generate.ts(1 hunks)
🔇 Additional comments (1)
src/server/routes/contract/extensions/erc1155/read/signature-generate.ts (1)
255-259: LGTM! Conditional signature generation logic is well-implemented.The conditional logic correctly handles the presence of
tokenIdby usinggenerateFromTokenIdwhen available, and falls back to the originalgeneratemethod otherwise. TheBigIntconversion fortokenIdis appropriate and maintains type safety.
| ) | ||
| : await contract.erc1155.signature.generate(payload); | ||
|
|
||
| console.log("signedPayload", signedPayload); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove debug console.log statement before production.
This debug statement should be removed as it can:
- Clutter production logs
- Potentially expose sensitive signature data
- Impact performance in high-throughput scenarios
- console.log("signedPayload", signedPayload);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| console.log("signedPayload", signedPayload); |
🤖 Prompt for AI Agents
In src/server/routes/contract/extensions/erc1155/read/signature-generate.ts at
line 261, remove the debug console.log statement that outputs the signedPayload
to prevent cluttering production logs, avoid exposing sensitive signature data,
and improve performance in high-throughput scenarios.
joaquim-verges
deleted the
Add_tokenId_support_to_ERC1155_signature_generation_and_remove_validityStartTimestamp
branch
Changes
How this PR will be tested
Output
(Example: Screenshot/GIF for UI changes, cURL output for API changes)
PR-Codex overview
This PR focuses on modifying the
erc1155SignatureGeneratefunction to enhance the signature generation process based on the presence oftokenId. It introduces conditional logic for generating a signed payload.Detailed summary
validityStartTimestampto aDateobject.tokenIdexists, callsgenerateFromTokenId.generatemethod.console.logstatement forsignedPayload.Summary by CodeRabbit
Bug Fixes
Chores