Remove ethers #1616
Remove ethers #1616
Conversation
WalkthroughThis pull request introduces a comprehensive migration from Changes
Sequence DiagramsequenceDiagram
participant Signer
participant Viem
participant KMS/PrivateKey
Signer->>Viem: signTypedData(domain, types, primaryType, message)
Viem->>Viem: hashTypedData(domain, types, primaryType, message)
Viem->>KMS/PrivateKey: Request signature
KMS/PrivateKey-->>Viem: Return signed data
Viem-->>Signer: Return signature
Possibly Related PRs
Suggested Reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 🪧 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 (
|
Test CoverageSummary
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/network/src/provider/utils/rpc-mapper/methods/eth_signTypedData_v4/eth_signTypedData_v4.ts (1)
77-77: Check for null or emptyprimaryTypevalues.When
typedData.primaryTypeis improperly defined or empty, the chain of typed data signing could fail. Consider adding a validation step or a fallback value ifprimaryTypeis unexpectedly missing.packages/aws-kms-adapter/tests/KMSVeChainSigner.unit.test.ts (1)
163-164: Resolution needed for null or incorrect typed data.The placeholders (
{} as unknown) might mask real type mismatches. Ensure correct fixture data is used rather than empty stubs to catch potential regressions.packages/network/src/signer/signers/types.d.ts (1)
372-375: Signature method refactored successfully.The newly added
primaryTypeandoptionsparameters harmonize with the updated typed data approach. Recommend verifying these optional parameters are properly handled in all signer implementations.packages/network/src/signer/signers/vechain-private-key-signer/vechain-private-key-signer.ts (1)
210-212: Documentation updated to match the new parameter names.
This helps users understand the updated typed data signing flow (primaryType,message).packages/aws-kms-adapter/src/KMSVeChainSigner.ts (1)
406-406: Expanded error context logs the new parameters.
IncludingprimaryTypeandmessagehelps troubleshoot typed data signing issues.packages/network/src/signer/signers/vechain-abstract-signer/vechain-abstract-signer.ts (1)
339-341: Comment changes align with the signature updates.
Renaming fromvaluetomessageclarifies parameter usage for typed data.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/aws-kms-adapter/src/KMSVeChainSigner.ts(2 hunks)packages/aws-kms-adapter/tests/KMSVeChainSigner.solo.test.ts(1 hunks)packages/aws-kms-adapter/tests/KMSVeChainSigner.unit.test.ts(3 hunks)packages/network/src/provider/utils/rpc-mapper/methods/eth_signTypedData_v4/eth_signTypedData_v4.ts(3 hunks)packages/network/src/signer/signers/types.d.ts(3 hunks)packages/network/src/signer/signers/vechain-abstract-signer/vechain-abstract-signer.ts(2 hunks)packages/network/src/signer/signers/vechain-private-key-signer/vechain-private-key-signer.ts(2 hunks)packages/network/tests/signer/signers/vechain-private-key-signer/vechain-private-key-signer.unit.test.ts(5 hunks)
🔇 Additional comments (19)
packages/network/src/provider/utils/rpc-mapper/methods/eth_signTypedData_v4/eth_signTypedData_v4.ts (2)
7-7: Good shift away from ethers to viem.
Removing TypedDataField references from ethers in favor of TypedDataParameter from viem makes the typed data approach more loosely coupled. This aligns with the PR's objective of removing ethers.
64-64: Ensure consistency in typed data usage.
The property types: Record<string, TypedDataParameter[]>; aligns with the new viem-based structure. Verify that all references to these data types across the codebase have been updated to avoid mismatched signatures.
packages/aws-kms-adapter/tests/KMSVeChainSigner.unit.test.ts (2)
8-8: Great job switching to viem.
This keeps the test suite consistent with the rest of the migration away from ethers.
139-139: Confirm typed data correctness.
The additional 'Mail' parameter ensures the primary type is specified. Confirm that all test fixtures are updated to support the new approach if more primary types are introduced.
packages/aws-kms-adapter/tests/KMSVeChainSigner.solo.test.ts (1)
302-302: Validate the primaryType parameter in integration tests.
Line 302 adds the primaryType argument. Confirm that integration tests cover cases where primaryType may be undefined or unexpected to ensure robust error handling.
packages/network/src/signer/signers/types.d.ts (2)
2-2: Migration to viem is consistent.
This new import from viem aligns with the codebase-wide migration goals to retire ethers.
184-184: Consider advanced usage of accessList.
accessList handling is mostly relevant to EVM-based networks. Verify if there’s any VeChain-specific nuance needed or if it remains purely for compatibility.
packages/network/tests/signer/signers/vechain-private-key-signer/vechain-private-key-signer.unit.test.ts (4)
25-25: Use of InvalidAbiEncodingTypeError is appropriate.
The explicit error type helps differentiate ABI encoding failures from generic errors.
294-297: Good to see the tests reflect the new parameter-based typed data approach.
Ensuring the test throws InvalidAbiEncodingTypeError is a clear indication that the signer’s error handling logic is now more specific.
Line range hint 318-327: Check for both new typed data structure and primary type usage.
Verifying exceptions with custom error messages ensures robust error handling. Confirm other references in the codebase also handle invalid typed data similarly for consistency.
348-348: Ethers compatibility check aligned with updated typed data approach.
This test confirms the signature’s consistency with ethers references, which offers reassurance about correctness. Great job maintaining backward compatibility!
packages/network/src/signer/signers/vechain-private-key-signer/vechain-private-key-signer.ts (3)
18-21: Imports from viem used to streamline typed data handling.
Replacing ethers-specific imports with viem fosters uniform data encoding and hashing. No issues noted.
218-220: Clear reflection of the refined typed data signature.
Switching from value to message clarifies usage. Ensure all code references are updated.
225-225: Using hashTypedData ensures robust domain separation.
This approach avoids manual hashing mistakes prevalent in raw hashing.
packages/aws-kms-adapter/src/KMSVeChainSigner.ts (3)
15-20: New imports from viem reflect the move away from ethers.
Including hashTypedData, recoverPublicKey, and toHex helps unify typed data operations.
392-394: primaryType argument integration.
The parameter alignment with VeChainPrivateKeySigner promotes consistency across signers.
398-398: Switch to hashTypedData.
Better clarity and standardization for typed data hashing, reducing reliance on ethers.
packages/network/src/signer/signers/vechain-abstract-signer/vechain-abstract-signer.ts (2)
11-11: Refined import from viem for typed data parameters.
Dropping ethers in favor of viem keeps the code consistent with the rest of the PR.
347-349: Updated abstract method signature to include primaryType.
All implementers now share consistent typed data parameters, reducing friction across signers.
There was a problem hiding this comment.
Looks good overhaul but two remarks:
- the tests should use mainnet genesis block id for the chainId:(EIP712 - Cannot pass a string as
chainIdwevm/abitype#256 (comment)) - there is no methods to infer the primaryType from the types
This is not the goal of the PR. We'll have another ticket for that. |
Summary by CodeRabbit
New Features
signTypedDatamethod to include aprimaryTypeparameter for better data handling.hashTypedDatafunction for improved signing process.Bug Fixes
signTypedDatato specifyInvalidAbiEncodingTypeErrorfor better error reporting.Documentation
Chores
etherslibrary withviemlibrary for typed data handling.