feat: implement cow shed sdk #247
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces extensive updates across multiple modules. The changes add clearer documentation and update method signatures (notably adding the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant CowShedSdk
participant CowShedHooks
participant Contract
User->>CowShedSdk: signCalls(args)
CowShedSdk->>CowShedHooks: getCowShedHooks(chainId, options)
CowShedSdk->>CowShedSdk: getNonce()
CowShedSdk->>Contract: send signed transaction
Contract-->>CowShedSdk: confirmation
CowShedSdk-->>User: transaction result
sequenceDiagram
participant Caller
participant getSigner
Caller->>getSigner: Call getSigner(signer)
alt Signer is a string (private key)
getSigner->>ethers.Wallet: Create new Wallet(signer)
else Signer has request/send method
getSigner->>Web3Provider: Initialize Web3Provider(signer)
Web3Provider-->>getSigner: Return signer instance
else Other object
getSigner-->>Caller: Return signer as is
end
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![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
🧹 Nitpick comments (9)
src/common/types/wallets.ts (1)
4-6: Wallet types provide good abstractions but could use stronger type constraintsThe type definitions for
PrivateKey,AccountAddress, andSignerLikeprovide useful abstractions for wallet-related concepts. TheSignerLikeunion type is particularly valuable as it enables flexible parameter typing for functions likegetSigner.Consider using TypeScript template literal types to enforce the character length constraints mentioned in the comments:
- export type PrivateKey = string // 64 characters + export type PrivateKey = `0x${string}` | `${string}` // Accepts both 0x-prefixed and raw format + // Further validation should be done at runtime - export type AccountAddress = `0x${string}` // 42 characters + export type AccountAddress = `0x${string & { length: 40 }}` // Enforces 0x + 40 hex charsNote: TypeScript's type system may not fully support exact string length validation, so runtime validation is still recommended for critical checks.
src/cow-shed/contracts/CoWShedHooks.spec.ts (1)
54-56: Hardcoded proxy creation code replaces constant reference.The test now directly includes the proxy creation code as a string literal instead of using a constant. While this makes the test more self-contained, it could make future updates more error-prone.
Consider keeping the proxy creation code in a constant for better maintainability. If the code changes in the future, you would only need to update it in one place.
- expect(defaultCowShed.proxyCreationCode()).toBe( - '0x60a034608e57601f61037138819003918201601f19168301916001600160401b038311848410176093578084926040948552833981010312608e57604b602060458360a9565b920160a9565b6080527f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc556040516102b490816100bd8239608051818181608f01526101720152f35b600080fd5b634e487b7160e01b600052604160045260246000fd5b51906001600160a01b0382168203608e5756fe60806040526004361015610018575b3661019457610194565b6000803560e01c908163025b22bc1461003b575063f851a4400361000e5761010d565b3461010a5760207ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffc36011261010a5773ffffffffffffffffffffffffffffffffffffffff60043581811691828203610106577f0000000000000000000000000000000000000000000000000000000000000000163314600014610101577f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc557fbc7cd75a20ee27fd9adebab32041f755214dbc6bffa90cc0225b39da2e5c2d3b8280a280f35b61023d565b8380fd5b80fd5b346101645760007ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffc360112610164576020610146610169565b73ffffffffffffffffffffffffffffffffffffffff60405191168152f35b600080fd5b333003610101577f000000000000000000000000000000000000000000000000000000000000000090565b60ff7f68df44b1011761f481358c0f49a711192727fb02c377d697bcb0ea8ff8393ac0541615806101ef575b1561023d5760046040517ff92ee8a9000000000000000000000000000000000000000000000000000000008152fd5b507f400ada75000000000000000000000000000000000000000000000000000000007fffffffff000000000000000000000000000000000000000000000000000000006000351614156101c0565b7f360894a13ba1a3210667c828492db98dca3e2076cc3735a920a3ca505d382bbc546000808092368280378136915af43d82803e1561027a573d90f35b3d90fdfea2646970667358221220c7c26ff3040b96a28e96d6d27b743972943aeaef81cc821544c5fe1e24f9b17264736f6c63430008190033' - ) + expect(defaultCowShed.proxyCreationCode()).toBe(PROXY_CREATION_CODE)Where
PROXY_CREATION_CODEwould be a constant defined at the top of the file or imported from a shared location.src/cow-shed/README.md (1)
17-19: Consider adding parameter documentation.The usage example shows how to call
getCowShedAccountbut doesn't explain what the parameters represent.Adding a brief comment explaining the parameters would improve the documentation:
// Get the cow-shed account for any given chainId and owner's address +// Parameters: +// - 1: Chain ID (Ethereum Mainnet) +// - '0xd8dA...': Owner's Ethereum address const cowShedAccount = cowShedSdk.getCowShedAccount(1, '0xd8dA6BF26964aF9D7eEd9e03E53415D37aA96045')src/cow-shed/contracts/CoWShedHooks.ts (1)
26-39: Centralize EIP-712 typed data definitions.
TheCOW_SHED_712_TYPESmatches standard EIP-712 definitions, but consider placing these definitions in a shared module if other parts of the codebase need them. This avoids possible duplication and promotes consistency.src/cow-shed/CowShedSdk.ts (3)
6-6: Check consistency in relative import paths.
You are importinggetSignerfrom'src/common/utils/wallet'while using relative imports (../) elsewhere. Consistency in import paths avoids confusion in large projects.- import { getSigner } from 'src/common/utils/wallet' + import { getSigner } from '../common/utils/wallet'
120-130: Caution with logging sensitive data.
Logging the full request during gas estimation failures might expose sensitive user data (e.g., function signatures). Consider masking or removing private details in production logs.- console.error('Error estimating gas for the cow-shed call: ' + JSON.stringify(factoryCall), error) + console.error('Error estimating gas for the cow-shed call.', error)
154-156: Potential nonce collisions.
Generating the nonce fromDate.now().toString()could collide if signCalls are triggered in quick succession. Consider appending a random suffix or using a more robust approach (like a counter).protected static getNonce(): string { - return ethers.utils.formatBytes32String(Date.now().toString()) + const timestamp = Date.now().toString() + const randomPart = Math.floor(Math.random() * 999999).toString() + return ethers.utils.formatBytes32String(`${timestamp}-${randomPart}`) }src/bridging/BridgingSdk.ts (2)
13-15: Minor grammatical improvement needed in class documentation.The class comment has a slight redundancy with "SDK for bridging for swapping tokens". Consider revising to "SDK for bridging and swapping tokens" or simply "SDK for bridging tokens between different chains."
/** - * SDK for bridging for swapping tokens between different chains. + * SDK for bridging and swapping tokens between different chains. */
39-41: Fix grammatical error in documentation.There's a grammatical issue in this JSDoc comment. It says "available sources networks" instead of "available source networks".
/** - * Get the available sources networks for the bridging. + * Get the available source networks for the bridging. */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
src/bridging/BridgingSdk.ts(2 hunks)src/common/index.ts(1 hunks)src/common/types/ethereum.ts(1 hunks)src/common/types/wallets.ts(1 hunks)src/common/utils/wallet.ts(1 hunks)src/cow-shed/CowShedSdk.ts(1 hunks)src/cow-shed/README.md(1 hunks)src/cow-shed/contracts/CoWShedHooks.spec.ts(2 hunks)src/cow-shed/contracts/CoWShedHooks.ts(1 hunks)src/cow-shed/contracts/utils.ts(1 hunks)src/cow-shed/index.ts(1 hunks)src/cow-shed/proxyInitCode.ts(0 hunks)src/cow-shed/types.ts(0 hunks)src/trading/getQuote.ts(2 hunks)src/trading/postLimitOrder.ts(1 hunks)src/trading/tradingSdk.ts(1 hunks)src/trading/types.ts(2 hunks)src/trading/utils.ts(1 hunks)
💤 Files with no reviewable changes (2)
- src/cow-shed/types.ts
- src/cow-shed/proxyInitCode.ts
🔇 Additional comments (23)
src/trading/postLimitOrder.ts (1)
6-6: Import path updated for shared getSigner utilityThe import path for the
getSignerfunction has been correctly updated to use the centralized implementation in the common module, maintaining consistent usage across the codebase.src/common/utils/wallet.ts (1)
1-15: Well-implemented utility function for signer managementThe implementation of
getSignerprovides an elegant solution for handling different types of signers (raw private keys, providers with Web3 interfaces, and direct Signer objects). This abstraction will simplify code elsewhere that needs to work with different signer types.The function correctly handles three distinct cases:
- Private key strings (creating an ethers Wallet)
- Provider objects with Web3 interface (using Web3Provider adapter)
- Direct Signer instances (passing through)
src/cow-shed/index.ts (1)
1-3:❓ Verification inconclusive
API consolidation aligns with SDK pattern, but consider backward compatibility
The change to export the new
CowShedSdkwhile removing exports forCoWShedHooks,contracts, andproxyInitCodeconsolidates the API into a more encapsulated SDK pattern, which aligns with the PR objective of simplifying interactions with the cow-shed module.This represents a breaking change for any code directly using the previously exported entities. Verify that all dependent code has been updated to use the new SDK, or consider maintaining backward compatibility by re-exporting these entities through the SDK.
🏁 Script executed:
#!/bin/bash # Search for imports of the removed exports echo "Searching for imports of removed exports..." rg -t ts "import.*from\s+['\"].*cow-shed['\"]" -A 3 | grep -E "(CoWShedHooks|contracts|proxyInitCode)"Length of output: 205
SDK Consolidation Change: Verify No Dependent Usage of Removed Entities
The consolidated export of
CowShedSdk(alongside./types) is consistent with the intended SDK simplification. However, removing the previous exports—namely,CoWShedHooks,contracts, andproxyInitCode—may cause issues for any code that still directly references them. An initial automated search targeting imports from the cow-shed package did not return any direct references, but this narrow search might not capture all dependent usages.Please manually verify that:
- There are no residual references to
CoWShedHooks,contracts, orproxyInitCodeanywhere in the repository (beyond just direct imports fromcow-shed).- All dependent modules have been updated to rely solely on the new
CowShedSdkAPI.If any legacy code still depends on the removed entities, consider either updating it to use the new SDK or re-exporting those entities temporarily to maintain backward compatibility.
Snippet (Lines 1–3 of
src/cow-shed/index.ts):export * from './types' export * from './CowShedSdk'src/trading/getQuote.ts (2)
17-17: Import path refactoring for better code organization.The import for
AccountAddresshas been refactored to use the common module path instead of local types. This aligns with the PR objective of relocating types from the trading SDK to a common directory for better reusability.
19-19:✅ Verification successful
Import update for centralized utility function.
The
getSignerfunction import has been updated to reflect its relocation to the common utilities module. This change supports the PR's goal of improving code organization and reusability.
🏁 Script executed:
#!/bin/bash # Verify that getSigner is properly exported from the common utils module rg -A 2 "export function getSigner" src/common/utils/wallet.tsLength of output: 188
Verified Import Update:
getSignerRelocation ConfirmedThe verification shows that
getSigneris correctly exported from the centralized utility module insrc/common/utils/wallet.ts. The import update insrc/trading/getQuote.ts(line 19) is valid and supports the PR's goals.src/cow-shed/contracts/CoWShedHooks.spec.ts (1)
4-5: Updated import paths align with the new directory structure.The import paths have been updated to reflect the reorganized directory structure. This is consistent with the PR's objectives of better code organization.
src/cow-shed/README.md (1)
1-58: Well-structured documentation for the new CowShed SDK.The README provides clear, comprehensive documentation explaining the purpose of the SDK and including well-structured usage examples. This aligns perfectly with the PR objective of introducing a new SDK for the cow-shed module.
src/trading/utils.ts (1)
1-1:✅ Verification successful
Simplified import statement reflecting code reorganization.
The import statement has been updated to only include the types that are actually used in this file after the
getSignerfunction was moved to a common utility module. This change supports the PR's goal of improving code organization.
🏁 Script executed:
#!/bin/bash # Verify that the getSigner function has been properly moved to the common module rg -A 5 "export function getSigner" src/common/utils/wallet.ts # Check for any remaining references to getSigner in this file that might have been missed rg "getSigner" src/trading/utils.tsLength of output: 320
Update Verified – Code Organization and Import Cleanup Confirmed
- Confirmed that the
getSignerfunction is now defined insrc/common/utils/wallet.ts.- Verified that no residual references to
getSignerremain insrc/trading/utils.ts.The simplified import statement, which now only includes the types actively used in this file, is consistent with the overall code reorganization efforts. Please proceed with the approval as these changes align with the PR’s goal of improving code modularity.
src/trading/types.ts (2)
12-12: Good adoption of unified signer type.
By importing and using theSignerLiketype, you improve consistency across different signer abstractions and ensure the code is easier to maintain.
75-75: Consistent with the new signer abstraction.
Replacing multiple signer-related union types withSignerLikekeeps the interface compact and clearer. No issues found.src/cow-shed/contracts/CoWShedHooks.ts (3)
18-19: Clean import reorganization.
The updated imports from../../commonkeep the references in a single place. This is a good step toward better modularity.
41-110: Thorough typed-data signing for multi-call logic.
ThesignCallsmethod effectively uses typed-data signing with EIP-712 or ETHSIGN. This is secure and flexible. Ensure that downstream code only accepts supported signing schemes to avoid user confusion.
23-25:❓ Verification inconclusive
Confirm correctness of proxy init code.
You’ve inlined the fullCOW_SHED_PROXY_INIT_CODEhex string. Verify that this matches the intended contract creation bytecode exactly, as any discrepancy will break proxy creation.
🏁 Script executed:
#!/bin/bash # Verify the correctness of the COW_SHED_PROXY_INIT_CODE by comparing it to the known contract creation bytecode # via strings or a known reference. (If there's a reference artifact, you can compare the hex. Otherwise, confirm from official docs.) echo "Please ensure you've validated that this hex code matches the official factory creation code from relevant resources."Length of output: 247
Action Required: Verify Proxy Init Code Accuracy
The inlined hex string in
src/cow-shed/contracts/CoWShedHooks.ts(lines 23-25) must be manually validated against the official contract creation bytecode. As noted, any discrepancy in theCOW_SHED_PROXY_INIT_CODEcan break proxy creation. Please confirm that this hex code exactly matches the intended factory creation code from your verified resources and documentation before proceeding.
- File:
src/cow-shed/contracts/CoWShedHooks.ts- Lines: 23-25
- Next Steps: Manually compare the inlined hex string against the official bytecode reference.
src/cow-shed/CowShedSdk.ts (2)
70-72: Allow additional signing schemes as needed.
Defaulting toEIP712is generally recommended. If you plan to allow other signing schemes (likeETHSIGN) in the future, consider clarifying usage constraints in docs. This will help prevent accidental insecure usage.
8-8:✅ Verification successful
Deadline usage.
UsingMaxUint256as a “never expires” deadline is typical, but confirm that all relevant chains and contract logic can handle such large deadlines without overflow or unintended reverts.
🏁 Script executed:
#!/bin/bash # This script checks references to chain-specific code that might handle large deadlines incorrectly. rg -A 5 "deadline"Length of output: 5684
Deadline Usage Verification – Approved
After reviewing references todeadlineacross the codebase, the use ofethers.constants.MaxUint256.toBigInt()insrc/cow-shed/CowShedSdk.tsis consistent with its treatment in related contract and test files. The deadline parameter is uniformly handled as auint256(bigint) across the SDK, hooks, and ABI definitions, and there’s no evidence of arithmetic or chain-specific logic that would cause overflow or unintended reverts. Based on our verification, usingMaxUint256as a "never expires" deadline is appropriate.src/bridging/BridgingSdk.ts (5)
7-9: Good documentation addition!Adding JSDoc comments to the interface properties improves code readability and helps other developers understand the purpose of each component.
32-34: Good documentation addition!Adding JSDoc comments to methods improves code readability and self-documentation.
46-49: Good documentation and signature update.Adding JSDoc comment and the
asynckeyword to this method improves code readability and correctly signals the asynchronous nature of the method to other developers.
53-59: Good documentation and signature update.Adding detailed JSDoc comment with parameter information and the
asynckeyword to this method improves code readability and correctly signals the asynchronous nature of the method.
63-66:❓ Verification inconclusive
Good documentation and signature update, but implementation is still missing.
The documentation addition and
asynckeyword are good improvements. However, note that this method still throws a "Not implemented yet!" error. Ensure this is addressed in a future PR if it's not within the scope of the current PR objectives.
🏁 Script executed:
#!/bin/bash # Check if the implementation of getQuote is addressed elsewhere in the codebase or if there's a ticket/issue for it echo "Checking for implementation of getQuote or related issues/tickets..." rg "bridging.*getQuote" --type ts -g "!*test*" -g "!*spec*" | grep -v "Not implemented yet"Length of output: 240
Action: Confirm and Document Future Implementation for
getQuoteMethod
- The updated documentation and signature (with the added
asynckeyword) are solid improvements.- However, the
getQuotemethod remains unimplemented as it currently throws a "Not implemented yet!" error.- If leaving it unimplemented in this PR is intentional, please add a clear TODO or reference an existing issue/ticket to track its future implementation.
src/common/index.ts (1)
7-7: Exports look good - wallet types are now available from common module.The addition of exporting wallet-related types from the common module is a good organizational change that aligns with the PR objective of relocating types to improve reusability.
src/trading/tradingSdk.ts (1)
13-13: Good refactoring - utility functions properly moved to common module.Moving the
getSignerfunction to the common module while keeping specific trading-related utilities in the trading module is a good architectural decision. This promotes better code organization and reusability across modules while maintaining appropriate boundaries.Also applies to: 17-17
src/cow-shed/contracts/utils.ts (1)
1-3: Import paths updated correctly.The import paths have been properly updated to reflect the new directory structure. These changes ensure that the CowShed module correctly references the generated contract interfaces from the common module.
|
@anxolin I added some tests for the SDK |
This PR implements a new SDK to simplify the use of cow-shed.
It hides some of the previously exposed types and utils from cow-shed module.
It exposes instead a
CowShedSdkwith only two methods:getCowShedAccount: Returns the cow-shed account owned by the provided addresssignCalls: Pre-authorize a multi-call from cow-shed account. It needs the owner's signer.Usage
To understand how to use these methods, you can check the README instructions:
Main changes in this PR
Moved some types/utils from trading SDK to common
See:
src/common/types/wallets.tssrc/common/utils/wallet.tsMainly, its to reuse the logic to get the signer from the tradingSDK into other SDKs. It defines also a type for simplicity:
type SignerLike = Signer | ExternalProvider | PrivateKeyImplemented CoW Shed SDK
src/cow-shed/CowShedSdk.ts: This SDK has the 2 methods explained above. The code should be self-documented.Summary by CodeRabbit
New Features
Documentation
Refactor
Tests