feat: Investigate and Explore Support for New Wallet API from JSON RPC Specification #515
Conversation
|
To view this pull requests documentation preview, visit the following URL: docs.page/focustree/starknet.dart~515 Documentation is deployed and generated using docs.page. |
|
""" WalkthroughThis change introduces a new Dart package, Changes
Sequence Diagram(s)sequenceDiagram
participant App as Dapp/Client
participant Provider as WalletProvider
participant RPC as JsonRpcClient
participant Wallet as Starknet Wallet (via HTTP)
App->>Provider: Call wallet method (e.g., requestAccounts)
Provider->>RPC: call(method, params)
RPC->>Wallet: HTTP POST (JSON-RPC)
Wallet-->>RPC: JSON-RPC response
RPC-->>Provider: Parse result or throw WalletError
Provider-->>App: Return typed result or error
Possibly related issues
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
🔭 Outside diff range comments (2)
packages/wallet_provider/dart_test.yaml (1)
1-3:⚠️ Potential issueCritical: Define test file patterns for unit tag
Currently theunittag has no associated file patterns, so tests won’t be discovered or executed under this tag. Add patterns to specify which files are considered unit tests.Apply this diff to fix the configuration:
-tags: - unit: +tags: + unit: + - 'test/**_test.dart'packages/wallet_provider/Implementation_notes.md (1)
30-32: 🛠️ Refactor suggestionExpand recommendations with security considerations
The recommendations section should include security considerations for using the Wallet API, especially regarding transaction signing and user interactions.
## 5. Recommendations * **Spec Clarification:** If possible, encourage clarification in the Wallet API specification regarding the expected structure of `params` for each method (positional list vs. object). * **`TypedData` Utilities:** Consider adding utility functions (potentially in `package:starknet` or `wallet_provider`) to facilitate the correct construction of `TypedData` objects, especially for String -> Felt conversion according to EIP-712/Starknet rules. +* **Security Guidelines:** Provide security guidelines for dApp developers using this package, including: + - Proper validation of transaction parameters before sending to wallets + - User confirmation UI/UX best practices + - Handling of sensitive data (e.g., avoiding logging of transaction details) + - HTTPS enforcement for all wallet API communications +* **Error Handling Patterns:** Document recommended error handling patterns for common failure scenarios: + - User rejected transactions + - Network failures + - Wallet disconnection + - Unsupported methods +* **Rate Limiting:** Consider implementing rate limiting for wallet API requests to prevent abuse and excessive prompts to users.
🧹 Nitpick comments (24)
packages/wallet_provider/.gitignore (1)
1-4: Dart ignore configuration is correct
The header comments reference Dart’s private-files guidelines, and excluding.dart_tool/prevents committing transient tool state.Consider also ignoring other generated artifacts such as the
.packagesfile, thebuild/directory, and any IDE-specific folders (e.g.,.idea/,*.iml) to keep the repository clean.packages/wallet_provider/.tool-versions (1)
1-1: Ensure Tool Versions Are Consistent Across the Project
Pinningstarknet-devnet 0.1.2here guarantees a stable devnet environment, but it’s unclear whether your Dart SDK (and potential Flutter SDK) versions are managed in a parent.tool-versions. Without a specified Dart version, contributors might end up on incompatible SDKs.
- Verify that the Dart (and Flutter, if applicable) SDK versions are defined elsewhere (e.g., the repo root).
- Optionally, add them here or include a comment header explaining this file’s scope.
packages/wallet_provider/lib/src/model/permission.dart (1)
1-5: Annotate enum for JSON serialization
To ensurePermissionvalues are correctly serialized/deserialized in snake_case (perbuild.yaml), add thejson_annotationdecorator before the enum. For example:import 'package:json_annotation/json_annotation.dart'; @JsonEnum(fieldRename: FieldRename.snake) enum Permission { accounts, }packages/wallet_provider/lib/src/model/invoke_call.dart (1)
14-16: Implement default for optionalcalldata
The TODO suggests defaultingcalldatato an empty list, but the field is currently nullable. Consider using Freezed’s@Default(<Felt>[])or@JsonKey(defaultValue: [])to enforce a non-null default and simplify downstream usage.packages/wallet_provider/lib/src/model/contract_class.g.dart (1)
9-26: Duplication observed between contract class implementations.The
ContractClassImplandSierraContractClassImplserialization methods (lines 9-26 and 28-47) are identical. This suggests either:
- They're meant to be distinct but currently share the same structure
- One class should be a type alias of the other
If these are meant to be the same structure with different names for semantic purposes, consider using a type alias or inheritance to reduce code duplication:
// In contract_class.dart: typedef SierraContractClass = ContractClass;This would simplify maintenance and reduce the generated code size.
packages/wallet_provider/lib/src/model/typed_data.dart (1)
11-24: Consider adding example usage for TypedData construction.This is a complex data structure that developers might struggle to construct correctly, especially with the dynamic typing in
StarknetType.Add documentation with an example or create a builder pattern to simplify construction:
// Example method to add to TypedData class static TypedData example() { return TypedData( types: { 'Person': [ {'name': 'name', 'type': 'felt'}, {'name': 'wallet', 'type': 'felt'}, ], }, primaryType: 'Person', domain: StarknetDomain(name: Felt.fromHex('0x01')), message: {'name': Felt.fromHex('0x01'), 'wallet': Felt.fromHex('0x02')}, ); }packages/wallet_provider/lib/src/model/starknet_chain.dart (2)
18-23: Implement URL fields using Uri instead of String.As noted in your TODOs, using
Uriwould provide better validation and functionality for URL handling.Consider implementing this improvement:
- @JsonKey(name: 'rpc_urls') - List<String>? rpcUrls, // TODO: Should be List<Uri>? + @JsonKey(name: 'rpc_urls', fromJson: _uriListFromJson, toJson: _uriListToJson) + List<Uri>? rpcUrls, - @JsonKey(name: 'block_explorer_url') - List<String>? blockExplorerUrl, // TODO: Should be List<Uri>? + @JsonKey(name: 'block_explorer_url', fromJson: _uriListFromJson, toJson: _uriListToJson) + List<Uri>? blockExplorerUrl, - @JsonKey(name: 'icon_urls') - List<String>? iconUrls, // TODO: Should be List<Uri>? + @JsonKey(name: 'icon_urls', fromJson: _uriListFromJson, toJson: _uriListToJson) + List<Uri>? iconUrls,And add these utility methods:
List<Uri>? _uriListFromJson(List<dynamic>? list) => list?.map((e) => Uri.parse(e.toString())).toList(); List<String>? _uriListToJson(List<Uri>? list) => list?.map((e) => e.toString()).toList();
10-28: Add helper methods for common Starknet chains.Creating method for common chains would improve developer experience and reduce errors.
Consider adding factory constructors:
// Add to StarknetChain class factory StarknetChain.mainnet() { return StarknetChain( id: 'SN_MAIN', chainId: Felt.fromHex('0x534e5f4d41494e'), chainName: 'Starknet Mainnet', rpcUrls: ['https://alpha-mainnet.starknet.io'], blockExplorerUrl: ['https://voyager.online'], // Add other fields as needed ); } factory StarknetChain.testnet() { // Similar implementation for testnet }packages/wallet_provider/lib/src/model/account_deployment_data.dart (3)
8-10: Fix typo in class documentation.The description contains a grammatical error.
-/// Description: Data required to deploy an account at and address +/// Description: Data required to deploy an account at an address
11-23: Add documentation for individual fields.The class has several fields that would benefit from more detailed explanations, especially for developers unfamiliar with Starknet account deployment.
Consider adding field-level documentation:
const factory AccountDeploymentData({ /// The Starknet address where the account will be deployed required Felt address, /// Hash of the account contract class to deploy @JsonKey(name: 'class_hash') required Felt classHash, /// Random value used to derive the account address required Felt salt, /// Constructor arguments for the account contract required List<Felt> calldata, /// Optional signature data for verification List<Felt>? sigdata, /// Indicates the Cairo version of the contract required DeploymentVersion version, }) = _AccountDeploymentData;
11-23: Consider adding validation for account deployment data.Account deployment is a critical operation, and ensuring data correctness is important.
Consider adding a validation method:
// Add to the AccountDeploymentData class bool validate() { // Check if address is non-zero if (address == Felt.zero) { return false; } // Check if classHash is non-zero if (classHash == Felt.zero) { return false; } // Add more validation as needed return true; }packages/wallet_provider/example/wallet_provider_example.dart (3)
63-67: Consider adding more robust chain switching logic.The chain ID validation is good, but consider extending this with actual chain switching functionality.
if (chainId != sepoliaChainId) { print('Warning: Connected to an unexpected chain ID!'); // Potentially try to switch chain using provider.switchStarknetChain(sepoliaChainId) + // Example chain switching logic: + try { + await provider.switchStarknetChain(sepoliaChainId); + print('Successfully switched to Sepolia'); + } catch (e) { + print('Failed to switch chains: $e'); + print('Please switch to Sepolia manually in your wallet'); + } }
84-97: Consider extracting complex parameter objects to variables.The
InvokeCallconstruction with nested parameters could be more readable if the parameters were extracted to variables.try { + // Define recipient address + final recipientAddress = felt( + '0x01234567890abcdef1234567890abcdef1234567890abcdef1234567890abcde'); + + // Define token amount (1 token with 18 decimals) + final amountLowPart = felt('0xDE0B6B3A7640000'); // 1 * 10^18 + final amountHighPart = felt('0x0'); // High part is 0 + final calls = [ InvokeCall( // Replace with a known contract address on Sepolia (e.g., an ERC20 contract) contractAddress: felt( '0x049d36570d4e46f48e99674bd3fcc84644ddd6b96f7c741b1562b82f9e004dc7'), entryPoint: 'transfer', // Assuming ERC20 transfer - calldata: [ - // Replace with a valid recipient address on Sepolia - felt( - '0x01234567890abcdef1234567890abcdef1234567890abcdef1234567890abcde'), - // Replace with actual amount low/high parts (e.g., 1 token with 18 decimals) - felt('0xDE0B6B3A7640000'), // 1 * 10^18 - felt('0x0'), // High part is 0 - ]) + calldata: [recipientAddress, amountLowPart, amountHighPart]) ];
114-148: Consider extracting the TypedData example to a separate helper function.The
TypedDataconstruction is complex and could be extracted to improve readability.try { print('Preparing typed data...'); - final typedData = TypedData( - types: { - "StarknetDomain": [ - {"name": "name", "type": "shortstring"}, - {"name": "version", "type": "felt"}, - {"name": "chainId", "type": "felt"}, - ], - "Person": [ - {"name": "name", "type": "shortstring"}, - {"name": "wallet", "type": "felt"} - ], - "Mail": [ - {"name": "from", "type": "Person"}, - {"name": "to", "type": "Person"}, - {"name": "contents", "type": "shortstring"} - ] - }, - primaryType: "Mail", - domain: StarknetDomain( - name: Felt.fromString("Example DApp"), - version: Felt.fromInt(1), - chainId: sepoliaChainId), - message: { - "from": { - "name": Felt.fromString("Alice"), - "wallet": felt( - "0x0111111111111111111111111111111111111111111111111111111111111111") - }, - "to": { - "name": Felt.fromString("Bob"), - "wallet": felt( - "0x0222222222222222222222222222222222222222222222222222222222222222") - }, - "contents": Felt.fromString("Hello Bob!") - }); + final typedData = createExampleTypedData(sepoliaChainId); print('Requesting signature...'); final signature = await provider.signTypedData(typedData); print('Data signed. Signature: $signature'); } catch (e) { print('Error signing typed data: $e'); } // Add this helper function at the end of the file + TypedData createExampleTypedData(Felt chainId) { + return TypedData( + types: { + "StarknetDomain": [ + {"name": "name", "type": "shortstring"}, + {"name": "version", "type": "felt"}, + {"name": "chainId", "type": "felt"}, + ], + "Person": [ + {"name": "name", "type": "shortstring"}, + {"name": "wallet", "type": "felt"} + ], + "Mail": [ + {"name": "from", "type": "Person"}, + {"name": "to", "type": "Person"}, + {"name": "contents", "type": "shortstring"} + ] + }, + primaryType: "Mail", + domain: StarknetDomain( + name: Felt.fromString("Example DApp"), + version: Felt.fromInt(1), + chainId: chainId), + message: { + "from": { + "name": Felt.fromString("Alice"), + "wallet": felt( + "0x0111111111111111111111111111111111111111111111111111111111111111") + }, + "to": { + "name": Felt.fromString("Bob"), + "wallet": felt( + "0x0222222222222222222222222222222222222222222222222222222222222222") + }, + "contents": Felt.fromString("Hello Bob!") + } + ); + }packages/wallet_provider/lib/src/model/asset.dart (1)
23-25: Add validation for TOKEN_SYMBOL constraintsThe comment indicates that TOKEN_SYMBOL constraints aren't enforced by the type system. Consider adding validation logic to ensure symbols follow standard conventions.
// Example validation method that could be added: factory AssetOptions.validated({ required Felt address, String? symbol, num? decimals, String? image, String? name, }) { if (symbol != null) { // Validate symbol (e.g., length, allowed characters) if (symbol.length > 11 || !RegExp(r'^[A-Z0-9.]+$').hasMatch(symbol)) { throw ArgumentError('Invalid TOKEN_SYMBOL format'); } } return AssetOptions( address: address, symbol: symbol, decimals: decimals, image: image != null ? Uri.parse(image) : null, name: name, ); }packages/wallet_provider/test/provider_test.dart (1)
6-9: Improve test environment configurationThe test setup uses a hardcoded fallback endpoint. Consider adding a more explicit configuration mechanism.
- final testUri = Uri.parse(Platform.environment['STARKNET_RPC'] ?? - 'https://free-rpc.nethermind.io/sepolia-juno'); + // Define supported networks and their endpoints + const supportedNetworks = { + 'sepolia': 'https://free-rpc.nethermind.io/sepolia-juno', + 'mainnet': 'https://rpc.starknet.rs/mainnet', + }; + + // Default to sepolia test network if not specified + final network = Platform.environment['STARKNET_NETWORK'] ?? 'sepolia'; + final testUri = Uri.parse(Platform.environment['STARKNET_RPC'] ?? + supportedNetworks[network] ?? supportedNetworks['sepolia']!);packages/wallet_provider/Implementation_notes.md (1)
18-23: Add examples to clarify key pointsThe key points section would benefit from concrete examples, especially for the more complex topics like parameter ambiguity and TypedData construction.
## 3. Discoveries and Key Points * **Wallet API vs. Standard RPC API Distinction:** The Wallet API is a distinct set of JSON-RPC methods that does not replace the standard RPC API but complements it for interactions requiring user wallet action or information. * **Model Generation:** The OpenRPC specification (`wallet_rpc.json`) is structured enough to allow for the generation (here, manually assisted) of type-safe Dart data models using `freezed`. * **Dependency on External Types:** The implementation heavily relies on types defined in `package:starknet` (`Felt`, `EntryPointsByType`, etc.). Version compatibility and the clarity of exports from this package are crucial. * **RPC Parameter Ambiguity:** The exact expected structure for parameters (`params` in the JSON-RPC request) is not always explicitly defined in the spec as being a positional list or a single object. The current implementation makes assumptions based on the order and apparent structure in `wallet_rpc.json`, but this might require adjustments based on actual wallet implementations. + **Example:** + ```dart + // Option 1: Parameters as positional list + final params = [accountAddress, calls, maxFee]; + + // Option 2: Parameters as named object + final params = { + 'account_address': accountAddress, + 'calls': calls, + 'max_fee': maxFee, + }; + ``` * **`signTypedData` Complexity:** Signing typed data requires careful construction of the `TypedData` object, especially converting all message and domain values into their corresponding `Felt` type (short string encoding, integer/hex conversion, etc.) to match the types defined in the `types` structure. + **Example:** + ```dart + final typedData = TypedData( + types: { + 'StarkNetDomain': [ + TypeMember(name: 'name', type: 'string'), + TypeMember(name: 'version', type: 'felt'), + TypeMember(name: 'chainId', type: 'felt'), + ], + 'Message': [ + TypeMember(name: 'sender', type: 'felt'), + TypeMember(name: 'amount', type: 'felt'), + ], + }, + primaryType: 'Message', + domain: { + 'name': shortStringFelt('My dApp'), + 'version': Felt.fromInt(1), + 'chainId': Felt.fromHexString('0x534e5f474f45524c49'), + }, + message: { + 'sender': Felt.fromHexString('0x123...'), + 'amount': Felt.fromInt(1000), + }, + ); + ```packages/wallet_provider/lib/src/model/contract_class.dart (1)
18-37: Duplicate data classes risk divergence
ContractClassandSierraContractClasscontain the exact same fields. Maintaining both guarantees future drift (e.g. when you later add anabiHashfield and forget one side). Prefer a single canonical representation to keep the public surface small and coherent.If you really need two aliases, let one extend the other or use a
typedef, but avoid duplicating Freezed classes.packages/wallet_provider/lib/src/model/wallet_error.dart (2)
19-25: Optimize theknownErrorCodegetter implementation.The current implementation uses a try-catch block to handle the potential exception when no matching error code is found. This approach is less efficient than using a direct lookup or a more functional approach.
Consider refactoring to use a more efficient lookup:
WalletErrorCode? get knownErrorCode { - try { - return WalletErrorCode.values.firstWhere((e) => e.code == code); - } catch (_) { - return null; // Not found - } + return WalletErrorCode.values.cast<WalletErrorCode?>().firstWhere( + (e) => e?.code == code, + orElse: () => null, + ); }Or even more concisely:
WalletErrorCode? get knownErrorCode => WalletErrorCode.values.where((e) => e.code == code).firstOrNull;
73-99: Simplify thecodegetter inWalletErrorCode.The
codegetter implementation is verbose with a switch statement mapping each enum value to its integer code. Since you're already using@JsonValueannotations, you could potentially leverage them directly.If possible, consider a more concise implementation:
// Option 1: Create a static map for lookup static final Map<WalletErrorCode, int> _codes = { WalletErrorCode.notERC20: 111, WalletErrorCode.unlistedNetwork: 112, // ... and so on }; int get code => _codes[this]!; // Option 2: If working with Dart 3.0+, you could consider using enhanced enums // with fields for the code valueThe comments in the code suggest that directly accessing the JsonValue annotation would be ideal but might not be possible, so one of these alternatives could be cleaner.
packages/wallet_provider/README.md (2)
1-5: Convert bare URL to Markdown linkA bare URL can break automatic link detection and violates MD034.
Replace with bracketed Markdown syntax:-This package is based on the official JSON-RPC specification available at: https://github.com/starkware-libs/starknet-specs/tree/master/wallet-api +This package is based on the official JSON-RPC specification available at: +[Starknet Wallet-API specification](https://github.com/starkware-libs/starknet-specs/tree/master/wallet-api)🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Bare URL used
null(MD034, no-bare-urls)
11-27: Fix unordered-list indentationThe list is indented with four spaces instead of two, tripping MD007.
Indent by two spaces to restore standard Markdown rendering.-* **Information & Permissions:** - * `supportedWalletApi()`: … +* **Information & Permissions:** + * `supportedWalletApi()`: …(and similarly for the rest of the nested items)
🧰 Tools
🪛 LanguageTool
[uncategorized] ~13-~13: Loose punctuation mark.
Context: ...API versions. *supportedSpecs(): Get supported Starknet JSON-RPC spec ve...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...pec versions. *getPermissions(): Get existing permissions for the Dapp. ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...or the Dapp. *requestAccounts(): Request wallet account addresses. *...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...nt addresses. *requestChainId(): Request the current chain ID from the w...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~19-~19: Loose punctuation mark.
Context: ...account. *switchStarknetChain(): Request to switch the wallet's active S...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~20-~20: Loose punctuation mark.
Context: ...et network. *addStarknetChain(): Request to add a new Starknet network c...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...on to the wallet. *watchAsset(): Request to add an ERC20 token to the wa...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~24-~24: Loose punctuation mark.
Context: ...ction. *addDeclareTransaction(): Request the wallet to sign and submit a...(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
12-12: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
18-18: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
19-19: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
20-20: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
21-21: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
23-23: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
24-24: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
26-26: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
packages/wallet_provider/lib/src/provider.dart (2)
85-100: Avoidloggeror propagate the errorUsing
- print('Warning: Unknown permission received: $p'); + // Replace with your preferred logging mechanism + // log.warning('Unknown permission received from wallet: $p');(or throw if strictness is required)
142-153: Parameter packing forwallet_switchStarknetChainis brittleRelying on positional placeholders (
null) can break if the spec adds or re-orders parameters. The spec examples also allow passing a single object.Simpler & safer approach:
- // Sticking to pure positional: [chainId, silent_mode?, api_version?] - final List<dynamic> methodParams = [chainId.toHexString()]; - // Only add subsequent parameters if they are provided - if (silentMode != null) { - methodParams.add(silentMode); - if (apiVersion != null) { - methodParams.add(apiVersion); - } - } else if (apiVersion != null) { - methodParams.add(null); // Add placeholder for silent_mode - methodParams.add(apiVersion); - } + final methodParams = [ + { + 'chain_id': chainId.toHexString(), + if (silentMode != null) 'silent_mode': silentMode, + if (apiVersion != null) 'api_version': apiVersion, + } + ];This follows the “single object” style used by many Wallet-API examples and avoids positional confusion. Confirm with the spec & wallet implementation before merging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
packages/wallet_provider/.gitignore(1 hunks)packages/wallet_provider/.tool-versions(1 hunks)packages/wallet_provider/CHANGELOG.md(1 hunks)packages/wallet_provider/Implementation_notes.md(1 hunks)packages/wallet_provider/LICENSE(1 hunks)packages/wallet_provider/README.md(1 hunks)packages/wallet_provider/analysis_options.yaml(1 hunks)packages/wallet_provider/build.yaml(1 hunks)packages/wallet_provider/dart_test.yaml(1 hunks)packages/wallet_provider/example/wallet_provider_example.dart(1 hunks)packages/wallet_provider/lib/src/index.dart(1 hunks)packages/wallet_provider/lib/src/model/account_deployment_data.dart(1 hunks)packages/wallet_provider/lib/src/model/account_deployment_data.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/account_deployment_data.g.dart(1 hunks)packages/wallet_provider/lib/src/model/add_declare_transaction_result.dart(1 hunks)packages/wallet_provider/lib/src/model/add_declare_transaction_result.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/add_declare_transaction_result.g.dart(1 hunks)packages/wallet_provider/lib/src/model/add_invoke_transaction_result.dart(1 hunks)packages/wallet_provider/lib/src/model/add_invoke_transaction_result.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/add_invoke_transaction_result.g.dart(1 hunks)packages/wallet_provider/lib/src/model/api_version.dart(1 hunks)packages/wallet_provider/lib/src/model/asset.dart(1 hunks)packages/wallet_provider/lib/src/model/asset.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/asset.g.dart(1 hunks)packages/wallet_provider/lib/src/model/contract_class.dart(1 hunks)packages/wallet_provider/lib/src/model/contract_class.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/contract_class.g.dart(1 hunks)packages/wallet_provider/lib/src/model/declare_txn_wallet.dart(1 hunks)packages/wallet_provider/lib/src/model/declare_txn_wallet.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/declare_txn_wallet.g.dart(1 hunks)packages/wallet_provider/lib/src/model/deployment_version.dart(1 hunks)packages/wallet_provider/lib/src/model/index.dart(1 hunks)packages/wallet_provider/lib/src/model/invoke_call.dart(1 hunks)packages/wallet_provider/lib/src/model/invoke_call.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/invoke_call.g.dart(1 hunks)packages/wallet_provider/lib/src/model/permission.dart(1 hunks)packages/wallet_provider/lib/src/model/starknet_chain.dart(1 hunks)packages/wallet_provider/lib/src/model/starknet_chain.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/starknet_chain.g.dart(1 hunks)packages/wallet_provider/lib/src/model/typed_data.dart(1 hunks)packages/wallet_provider/lib/src/model/typed_data.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/typed_data.g.dart(1 hunks)packages/wallet_provider/lib/src/model/wallet_error.dart(1 hunks)packages/wallet_provider/lib/src/model/wallet_error.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/wallet_error.g.dart(1 hunks)packages/wallet_provider/lib/src/provider.dart(1 hunks)packages/wallet_provider/lib/wallet_provider.dart(1 hunks)packages/wallet_provider/pubspec.yaml(1 hunks)packages/wallet_provider/test/melos_test.dart(1 hunks)packages/wallet_provider/test/provider_test.dart(1 hunks)
🧰 Additional context used
🪛 LanguageTool
packages/wallet_provider/README.md
[uncategorized] ~13-~13: Loose punctuation mark.
Context: ...API versions. * supportedSpecs(): Get supported Starknet JSON-RPC spec ve...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...pec versions. * getPermissions(): Get existing permissions for the Dapp. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...or the Dapp. * requestAccounts(): Request wallet account addresses. *...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...nt addresses. * requestChainId(): Request the current chain ID from the w...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~19-~19: Loose punctuation mark.
Context: ...account. * switchStarknetChain(): Request to switch the wallet's active S...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~20-~20: Loose punctuation mark.
Context: ...et network. * addStarknetChain(): Request to add a new Starknet network c...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...on to the wallet. * watchAsset(): Request to add an ERC20 token to the wa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~24-~24: Loose punctuation mark.
Context: ...ction. * addDeclareTransaction(): Request the wallet to sign and submit a...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~88-~88: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...he WalletProvider methods have clear, strongly-typed signatures for both input parameters an...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
🪛 markdownlint-cli2 (0.17.2)
packages/wallet_provider/README.md
3-3: Bare URL used
null
(MD034, no-bare-urls)
12-12: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
18-18: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
19-19: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
20-20: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
21-21: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
23-23: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
24-24: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
26-26: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (64)
packages/wallet_provider/.gitignore (1)
5-7: Excludepubspec.lockfor library packages
Omittingpubspec.lockin library packages aligns with Dart’s recommendation to let consumers manage their own dependency resolutions.packages/wallet_provider/LICENSE (1)
1-21: Approve MIT license file
The MIT license text is correctly applied, and all required clauses are present.packages/wallet_provider/CHANGELOG.md (1)
1-3: Initial changelog entry is present
TheCHANGELOG.mdcorrectly documents the first release (0.0.1+1) with an initial entry.packages/wallet_provider/lib/src/index.dart (1)
1-2: Approve aggregation of exports
Re-exportingmodel/index.dartandprovider.dartprovides a clean entry point and simplifies imports for consumers.packages/wallet_provider/analysis_options.yaml (1)
1-7: Approve analysis options configuration
The file correctly includes the recommended lint rules and suppresses expected analyzer errors (invalid_annotation_target,non_constant_identifier_names) for generated code.packages/wallet_provider/lib/wallet_provider.dart (1)
5-5: Re-exports look good
The exports forsrc/provider.dartandsrc/model/index.dartcorrectly expose the public API of the package.Also applies to: 8-8
packages/wallet_provider/build.yaml (1)
1-10: Build configuration for code generation
Thebuild.yamlproperly configures thefreezedandjson_serializablebuilders with the intended options (union_key,explicit_to_json,field_rename). No issues detected.packages/wallet_provider/test/melos_test.dart (1)
1-6: Dummy test to satisfy Melos tagging
This empty test ensures Melos finds at least one test tagged as'unit'. It correctly uses thetagsparameter and will always pass.packages/wallet_provider/lib/src/model/api_version.dart (1)
1-4: Typedef for API version is appropriate
TheApiVersionalias forStringclearly documents intent, and the doc comment provides context for future validation. Ready to merge.packages/wallet_provider/lib/src/model/deployment_version.dart (1)
1-9: DeploymentVersion enum correctly defined with JsonValue annotations.
The enum cleanly represents Cairo versions 0 and 1 and will serialize/deserialize via the provided@JsonValuemetadata. No issues detected.packages/wallet_provider/lib/src/model/index.dart (1)
1-14: Centralized model exports are clear and comprehensive.
This barrel file cleanly re-exports all Wallet API models, simplifying imports for consumers of the package. No issues detected.packages/wallet_provider/lib/src/model/add_invoke_transaction_result.dart (2)
1-6: Freezed model setup for invoke transaction result is correct.
Imports, part directives, and the@freezedannotation are properly configured for code generation.
7-17: Field mapping and type usage are appropriate.
ThetransactionHashfield uses the proper JSON key and theFelttype fromstarknet. Serialization via the generatedfromJson/toJsonmethods will work seamlessly.packages/wallet_provider/lib/src/model/declare_txn_wallet.dart (2)
1-7: Model imports and part directives are well-configured.
The imports forfreezed_annotation,starknet, and the localcontract_class.dart, along with thepartstatements, align with standardfreezedpatterns.
11-20: DeclareTxnWallet fields and JSON mappings align with specification.
FieldscompiledClassHash,classHash, andcontractClasshave the correct types (FeltandContractClass) and JSON key mappings. No issues detected.packages/wallet_provider/lib/src/model/add_declare_transaction_result.g.dart (1)
1-22: Generated serialization code appears correct.
This file is auto-generated byjson_serializable; no manual edits are required.packages/wallet_provider/lib/src/model/wallet_error.g.dart (2)
9-14: Generated JSON deserialization is correct. The_$$WalletErrorImplFromJsonfunction properly converts the JSON map into a_WalletErrorImplinstance, parsingcode,message, anddataas expected.
16-21: Generated JSON serialization is correct. The_$$WalletErrorImplToJsonmethod accurately serializes the_WalletErrorImplinstance back into JSON with the appropriate field names.packages/wallet_provider/pubspec.yaml (4)
1-4: Validate package metadata. The package name, description, version, and repository fields are correctly set for the newwallet_providerpackage.
6-8: Review environment SDK constraint. Pinning the Dart SDK to^3.0.5aligns with the dependencies (freezed_annotation,json_annotation, etc.) and ensures compatibility.
9-14: Review runtime dependencies. The declared dependencies (starknet,freezed_annotation,http,json_annotation) are appropriate for implementing the Wallet API client, providing core types, JSON serialization, and HTTP communication.
15-20: Review dev_dependencies. The development dependencies (build_runner,freezed,json_serializable,lints,test) cover code generation, linting, and testing requirements.packages/wallet_provider/lib/src/model/add_declare_transaction_result.dart (2)
1-5: Imports and part directives are correct. The file properly importsfreezed_annotationandstarknet, and includes the necessarypartdirectives forfreezedand JSON generated code.
7-19: Model declaration is accurate. TheAddDeclareTransactionResultclass uses@freezedand@JsonKeyto map JSON fields (transaction_hash,class_hash) toFelt-typed properties, and includes afromJsonfactory. This aligns with the Wallet API specification.packages/wallet_provider/lib/src/model/declare_txn_wallet.g.dart (2)
9-16: Generated JSON deserialization is correct. The_$$DeclareTxnWalletImplFromJsonfunction accurately constructs aDeclareTxnWalletinstance by invokingFelt.fromJsonandContractClass.fromJsonon the respective JSON fields.
18-24: Generated JSON serialization is correct. The_$$DeclareTxnWalletImplToJsonmethod serializes theDeclareTxnWalletinstance back into a JSON map using.toJson()on each field.packages/wallet_provider/lib/src/model/add_invoke_transaction_result.g.dart (2)
9-13: Generated JSON deserialization is correct. The_$$AddInvokeTransactionResultImplFromJsonfunction cleanly parses thetransaction_hashfield into aFeltinstance usingFelt.fromJson.
15-19: Generated JSON serialization is correct. The_$$AddInvokeTransactionResultImplToJsonmethod returns a JSON map withtransaction_hashserialized viatoJson().packages/wallet_provider/lib/src/model/starknet_chain.g.dart (2)
9-26: Generated deserialization logic is correct
The_$$StarknetChainImplFromJsonfunction accurately maps JSON fields (id,chain_id,chain_name,rpc_urls,block_explorer_url,native_currency,icon_urls) to the Dart model, including null-safe handling of optional lists and nestedAsset.fromJson.
28-37: Generated serialization logic is correct
The_$$StarknetChainImplToJsonfunction serializes the Dart model back to JSON with appropriate keys and usestoJson()forFeltandAsset, matching the Wallet API spec.packages/wallet_provider/lib/src/model/account_deployment_data.g.dart (2)
9-22: Generated deserialization logic is correct
The_$$AccountDeploymentDataImplFromJsonfunction decodes required fields (address,class_hash,salt,calldata,version) and optionalsigdata, correctly invokingFelt.fromJsonand enum decoding.
24-33: Generated serialization logic is correct
The_$$AccountDeploymentDataImplToJsonfunction maps the Dart instance to JSON with the expected structure, including convertingFeltand encoding theversionvia_DeploymentVersionEnumMap.packages/wallet_provider/lib/src/model/invoke_call.dart (2)
1-6: Imports and part directives are correctly configured
Imports offreezed_annotationandstarknetplus thepartdirectives follow Freezed conventions.
9-20: Freezed model setup looks good
TheInvokeCallfactory correctly maps JSON keyscontract_addressandentry_pointto typed properties, and Freezed will generate the appropriate serialization code.packages/wallet_provider/lib/src/model/typed_data.g.dart (4)
9-21: TypedData deserialization is correct
The_$$TypedDataImplFromJsonfunction transformstypes,primary_type,domain, andmessagefields accurately, preserving nested structures and invokingStarknetDomain.fromJsonwhere needed.
23-30: TypedData serialization is correct
The_$$TypedDataImplToJsonfunction serializes the instance back to JSON with the correct field names and nested structures.
31-42: StarknetDomain deserialization is correct
The_$$StarknetDomainImplFromJsonhandles optionalFeltfields (name,version,chain_id,revision) with null checks, aligning with the null-safe model.
44-52: StarknetDomain serialization is correct
The_$$StarknetDomainImplToJsonmaps all optional fields using?.toJson(), matching the JSON schema for the domain object.packages/wallet_provider/lib/src/model/invoke_call.g.dart (2)
9-16: InvokeCall deserialization is correct
The_$$InvokeCallImplFromJsonfunction properly handles requiredcontract_addressandentry_point, and null-safe mapping of optionalcalldata.
18-23: InvokeCall serialization is correct
The_$$InvokeCallImplToJsonfunction serializesInvokeCallback to JSON with the expected keys, includingtoJson()onFeltand handling nullablecalldata.packages/wallet_provider/lib/src/model/contract_class.g.dart (1)
50-67: LGTM: Proper handling of deprecated contract class.The
DeprecatedContractClassImplserialization properly handles the different structure with a stringprograminstead ofsierraProgram, different entry points type, and a list of ABI entries rather than a string.packages/wallet_provider/example/wallet_provider_example.dart (4)
1-10: Well-structured imports with proper hiding of conflicting types.The imports are well organized, with a clear explanation of why
TypedDatais hidden from the StarkNet package. The placeholder wallet URI with explanatory comments helps developers understand how to properly configure the connection in a real application.
11-32: Good helper functions for test data creation.The helper functions and constants provide useful shortcuts for creating test data. The
createDummyContractClassmethod particularly saves repetitive code while demonstrating the expected structure of a contract class.
39-55: Robust account request implementation with proper error handling.The account request implementation demonstrates proper error handling by:
- Using try/catch blocks
- Providing informative error messages
- Checking for empty accounts before proceeding
- Exiting early if accounts cannot be obtained
This is a great pattern for production code.
175-180: Good resource management with provider closing.The
finallyblock ensures that the provider is properly closed, which is a good practice for resource management. This prevents potential memory leaks or hanging connections.packages/wallet_provider/lib/src/model/account_deployment_data.freezed.dart (1)
1-300: Generated code follows best practices for immutable data models.This auto-generated code from Freezed correctly implements an immutable
AccountDeploymentDataclass with appropriate:
- JSON serialization/deserialization
- Deep equality checks
- Immutable collections with
EqualUnmodifiableListView- Proper handling of nullable fields
- Clean toString implementation
- Efficient hashCode implementation
The model structure includes all necessary fields for account deployment on Starknet with appropriate JSON key mapping for
classHashtoclass_hash.packages/wallet_provider/lib/src/model/asset.g.dart (1)
1-38: Generated serialization code handles complex types correctly.This auto-generated code correctly implements JSON serialization and deserialization for the ERC20 asset models:
ERC20AssetImpl: Handles the asset type and nested options objectAssetOptionsImpl: Properly transforms the specializedFelttype using its JSON methodsThe default type value of "ERC20" and optional fields (symbol, decimals, image, name) are handled appropriately according to the Starknet wallet API specifications.
packages/wallet_provider/lib/src/model/wallet_error.freezed.dart (1)
1-204: Error model follows standard JSON-RPC error structure.This generated code correctly implements a
WalletErrorclass following the standard JSON-RPC error structure with:
- Error code (integer)
- Error message (string)
- Optional additional data
The class extends a base
WalletErrorclass, likely with additional error handling capabilities defined in the base class. The implementation includes proper serialization/deserialization, equality checks, and copying methods expected of an immutable error model.This structure will enable clean error handling and reporting when interacting with StarkNet wallets via JSON-RPC.
packages/wallet_provider/lib/src/model/asset.dart (1)
10-17: LGTM: Well-structured sealed class implementationThe sealed class approach for
Assetwith a singleerc20variant is well-implemented and follows best practices for extensibility. Future token type variants can be added easily.packages/wallet_provider/lib/src/model/add_invoke_transaction_result.freezed.dart (1)
1-180: LGTM: Generated code for transaction result model looks correctThis is generated Freezed code that properly implements immutability and serialization for the
AddInvokeTransactionResultmodel. The JSON property mapping using@JsonKey(name: 'transaction_hash')correctly handles snake_case to camelCase conversion.packages/wallet_provider/Implementation_notes.md (2)
1-5: LGTM: Clear objective statementThe objective statement clearly defines the purpose of the implementation and how it fits into the broader Starknet ecosystem.
7-15: LGTM: Well-structured implementation approachThe implementation approach section provides a clear overview of the package structure, dependencies, and design decisions. This is helpful for maintaining the package in the future.
packages/wallet_provider/lib/src/model/add_declare_transaction_result.freezed.dart (1)
1-32: Generated file – can safely be ignoredThis file is 100 % Freezed-generated boilerplate. Unless we observe compilation errors (none spotted) or forbidden patterns, there is no need to review or hand-edit it.
packages/wallet_provider/lib/src/model/typed_data.freezed.dart (1)
1-40: Generated file – no manual action requiredLike the previous file this is pure Freezed output; manual edits will be overwritten. Any improvements (e.g. stronger typing for the
messagemap) need to be done intyped_data.dart, not here.packages/wallet_provider/lib/src/model/contract_class.dart (1)
21-33: Minor spec alignment – considerUint8Listfor bytecode
List<Felt> sierraProgramforces every Sierra instruction to be encoded as a Felt.
The upstream JSON-RPC spec represents the Sierra program as an array of field elements after padding/trimming. If raw bytecode is ever required (e.g. for local class-hash computation) the current modelling will make it harder. Evaluate whetherUint8Listor a wrapper value object would be more future-proof.packages/wallet_provider/lib/src/model/contract_class.freezed.dart (1)
1-20: Generated file – no manual reviewAll logic here is produced by Freezed and mirrors the source file; focus corrections in
contract_class.dartinstead.packages/wallet_provider/lib/src/model/starknet_chain.freezed.dart (1)
296-296: Improve equality comparisons for collections.The equality implementation correctly uses
DeepCollectionEqualityfor comparing lists, which avoids issues with reference equality for collections.packages/wallet_provider/lib/src/model/wallet_error.dart (2)
32-70: Good use of comprehensive error types and documentation.The error code enum is well-designed with clear naming, proper documentation, and explicit integer values using
@JsonValue. This will make error handling more predictable and maintainable.
102-147: Well-structured error message extension.The
WalletErrorCodeExtensionprovides a clean way to access standardized error messages and descriptions, following good separation of concerns.packages/wallet_provider/lib/src/model/declare_txn_wallet.freezed.dart (2)
23-28: Properly structured data model with consistent JSON key naming.The
DeclareTxnWalletmodel uses snake_case JSON keys that match the API specification, with appropriate@JsonKeyannotations.
51-52: Good usage of copyWith interface for contract class.The implementation correctly handles nested copyWith operations, which is important for complex models containing other class references like
ContractClass.packages/wallet_provider/lib/src/model/asset.freezed.dart (2)
162-164: Good use of default values for type field.The code correctly implements a default value for the
typefield inERC20Asset, which helps ensure consistent data representation.
22-57: Well-structured pattern matching for the Asset union type.The
Assetclass properly implements pattern matching methods (when,map,maybeWhen, etc.) which makes handling different asset types both type-safe and expressive.packages/wallet_provider/lib/src/model/invoke_call.freezed.dart (1)
1-226: Generated file – no manual changes expected
invoke_call.freezed.dartis Freezed–generated boilerplate. Manual edits will be lost the next time code-generation runs, so I’m skipping detailed review here.
| String? | ||
| symbol, // TOKEN_SYMBOL constraints (pattern/length) not enforced by type system | ||
| num? decimals, | ||
| String? image, // TODO: Should be Uri? |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Address TODO comment for image field type
The TODO comment about whether the image field should be a Uri type deserves resolution. Using Uri would provide type safety and validation for URL formats.
- String? image, // TODO: Should be Uri?
+ Uri? image,Consider updating related serialization in generated code accordingly:
// In generated _$AssetOptionsFromJson method:
image: json['image'] == null ? null : Uri.parse(json['image'] as String),
// In generated _$AssetOptionsToJson method:
'image': instance.image?.toString(),| group('WalletProvider Info Calls', () { | ||
| test('.supportedWalletApi() fails gracefully on standard node', () async { | ||
| try { | ||
| await provider.supportedWalletApi(); | ||
| fail('Should have thrown an error'); // Expecting failure | ||
| } catch (e) { | ||
| // Expecting a WalletError with code -32601 (Method Not Found) | ||
| expect(e, isA<WalletError>()); | ||
| if (e is WalletError) { | ||
| expect(e.code, equals(-32601)); | ||
| } | ||
| print( | ||
| "supportedWalletApi failed as expected against non-wallet endpoint: $e"); | ||
| } | ||
| }); | ||
|
|
||
| test('.supportedSpecs() fails gracefully on standard node', () async { | ||
| try { | ||
| await provider.supportedSpecs(); | ||
| fail('Should have thrown an error'); // Expecting failure | ||
| } catch (e) { | ||
| // Expecting a WalletError with code -32601 (Method Not Found) | ||
| expect(e, isA<WalletError>()); | ||
| if (e is WalletError) { | ||
| expect(e.code, equals(-32601)); | ||
| } | ||
| print( | ||
| "supportedSpecs failed as expected against non-wallet endpoint: $e"); | ||
| } | ||
| }); | ||
|
|
||
| // Let's try requestChainId - it might be implemented by some nodes/proxies | ||
| test('.requestChainId() returns chainId or fails gracefully', () async { | ||
| try { | ||
| final chainId = await provider.requestChainId(); | ||
| expect(chainId, isA<Felt>()); | ||
| // You could add a check for the specific chain ID if you know the endpoint | ||
| // final expectedChainId = Felt.fromHexString('YOUR_EXPECTED_CHAIN_ID_HEX'); | ||
| // expect(chainId, equals(expectedChainId)); | ||
| print( | ||
| "requestChainId succeeded with chainId: ${chainId.toHexString()}"); | ||
| } catch (e) { | ||
| // Expecting a WalletError with code -32601 (Method Not Found) if node doesn't support it | ||
| expect(e, isA<WalletError>()); | ||
| if (e is WalletError) { | ||
| expect(e.code, equals(-32601)); | ||
| } | ||
| print("requestChainId failed as expected for this endpoint: $e"); | ||
| } | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add positive test cases with mock wallet responses
The current tests only verify error handling against a standard node. Consider adding tests with mocked wallet responses to verify success paths.
Create a mock client that returns predefined responses to test success scenarios:
class MockWalletClient implements JsonRpcClient {
final Map<String, dynamic> mockResponses;
MockWalletClient(this.mockResponses);
@override
Future<dynamic> sendJsonRpc(String method, [dynamic params]) async {
if (mockResponses.containsKey(method)) {
return mockResponses[method];
}
throw WalletError(
code: -32601,
message: 'Method not found',
);
}
}
// Then in your test:
test('supportedWalletApi returns list of supported methods', () async {
final mockClient = MockWalletClient({
'wallet_supportedWalletApi': [
'wallet_requestAccounts',
'wallet_requestChainId',
'wallet_addInvokeTransaction'
]
});
final provider = WalletProvider.fromClient(mockClient);
final methods = await provider.supportedWalletApi();
expect(methods, isA<List<String>>());
expect(methods, contains('wallet_requestAccounts'));
expect(methods, contains('wallet_addInvokeTransaction'));
});| throw _privateConstructorUsedError; // TODO: Should be List<Uri>? | ||
| @JsonKey(name: 'block_explorer_url') | ||
| List<String>? get blockExplorerUrl => | ||
| throw _privateConstructorUsedError; // TODO: Should be List<Uri>? |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply consistent URL handling for block explorer URLs.
Similar to the rpcUrls field, blockExplorerUrl could benefit from using the Uri type instead of String.
In the source model file, update the type to use Uri with a custom JSON converter, similar to the recommendation for rpcUrls.
| String get chainName => throw _privateConstructorUsedError; | ||
| @JsonKey(name: 'rpc_urls') | ||
| List<String>? get rpcUrls => | ||
| throw _privateConstructorUsedError; // TODO: Should be List<Uri>? |
There was a problem hiding this comment.
🛠️ Refactor suggestion
TODO: Consider using Uri type for improved URL validation and handling.
The TODO comment suggests using List<Uri> instead of List<String> for rpcUrls. This is a good suggestion as the Uri type provides validation and methods specifically for working with URLs.
In the source model file that generates this code, you could update the type to use Uri with a custom JSON converter:
- @JsonKey(name: 'rpc_urls') List<String>? rpcUrls,
+ @JsonKey(name: 'rpc_urls', fromJson: _stringsToUris, toJson: _urisToStrings) List<Uri>? rpcUrls,Add these converter functions to the source file:
List<Uri>? _stringsToUris(List<dynamic>? strings) {
return strings?.map((s) => Uri.parse(s as String)).toList();
}
List<String>? _urisToStrings(List<Uri>? uris) {
return uris?.map((uri) => uri.toString()).toList();
}| Felt get address => throw _privateConstructorUsedError; | ||
| String? get symbol => | ||
| throw _privateConstructorUsedError; // TOKEN_SYMBOL constraints (pattern/length) not enforced by type system | ||
| num? get decimals => throw _privateConstructorUsedError; | ||
| String? get image => | ||
| throw _privateConstructorUsedError; // TODO: Should be Uri? | ||
| String? get name => throw _privateConstructorUsedError; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider adding validation for token symbols and better URL handling.
There are two improvements noted in the comments:
- The
TOKEN_SYMBOLconstraints aren't enforced by the type system - The
imageURL field should be aUritype
In the source model file:
- For token symbols, consider adding a validator or custom constructor that enforces the pattern/length constraints:
factory AssetOptions.validated({
required Felt address,
String? symbol,
num? decimals,
String? image,
String? name,
}) {
// Validate symbol if provided
if (symbol != null) {
// Add regex validation or length checks
if (symbol.length > MAX_SYMBOL_LENGTH) {
throw ArgumentError('Symbol exceeds maximum length');
}
if (!RegExp(r'^[A-Z0-9]+$').hasMatch(symbol)) {
throw ArgumentError('Symbol must only contain uppercase letters and digits');
}
}
return AssetOptions(
address: address,
symbol: symbol,
decimals: decimals,
image: image,
name: name,
);
}- For the image URL, use the
Uritype with custom JSON converters as suggested earlier for the chain URLs.
There was a problem hiding this comment.
TOKEN_SYMBOL constraints fixed based on wallet api
There was a problem hiding this comment.
@redDwarf03 Thanks for implementing the TOKEN_SYMBOL constraints based on the wallet API specifications! That's great to see.
Do you also have plans to address the image URL field suggestion in a future update, or is there a specific reason to keep it as a String type?
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (10)
packages/wallet_provider/README.md (10)
1-3: Use a proper markdown link for the spec URL
Instead of a bare URL (which triggers MD034), wrap it in angle brackets or convert it into a named link. For example:This package is based on the official JSON-RPC specification available at [Starknet Wallet API spec](https://github.com/starkware-libs/starknet-specs/tree/master/wallet-api).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Bare URL used
null(MD034, no-bare-urls)
11-16: Normalize nested list formatting under “Information & Permissions”
Current spacing is inconsistent. Consider:* **Information & Permissions:** * `supportedWalletApi()`: Get supported Wallet API versions. * `supportedSpecs()`: Get supported Starknet JSON-RPC spec versions. * `getPermissions()`: Get existing permissions for the Dapp. * `requestAccounts()`: Request wallet account addresses. * `requestChainId()`: Request the current chain ID from the wallet.Use two-space indents for subitems and a single space after the
*.🧰 Tools
🪛 LanguageTool
[uncategorized] ~13-~13: Loose punctuation mark.
Context: ...API versions. *supportedSpecs(): Get supported Starknet JSON-RPC spec ve...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...pec versions. *getPermissions(): Get existing permissions for the Dapp. ...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...or the Dapp. *requestAccounts(): Request wallet account addresses. *...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...nt addresses. *requestChainId(): Request the current chain ID from the w...(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
12-12: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
17-21: Align “Account & Network Management” list indentation
Refactor to:* **Account & Network Management:** * `deploymentData()`: Request data needed to deploy the connected account. * `switchStarknetChain()`: Request to switch the wallet’s active Starknet network. * `addStarknetChain()`: Request to add a new Starknet network configuration to the wallet. * `watchAsset()`: Request to add an ERC20 token to the wallet’s tracked assets.🧰 Tools
🪛 LanguageTool
[uncategorized] ~19-~19: Loose punctuation mark.
Context: ...account. *switchStarknetChain(): Request to switch the wallet's active S...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~20-~20: Loose punctuation mark.
Context: ...et network. *addStarknetChain(): Request to add a new Starknet network c...(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...on to the wallet. *watchAsset(): Request to add an ERC20 token to the wa...(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
18-18: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
19-19: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
20-20: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
21-21: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
22-24: Standardize “Transaction Submission” bullets
Use consistent two-space indent:* **Transaction Submission:** * `addInvokeTransaction()`: Request the wallet to sign and submit an invoke transaction. * `addDeclareTransaction()`: Request the wallet to sign and submit a declare transaction.🧰 Tools
🪛 LanguageTool
[uncategorized] ~24-~24: Loose punctuation mark.
Context: ...ction. *addDeclareTransaction(): Request the wallet to sign and submit a...(UNLIKELY_OPENING_PUNCTUATION)
🪛 markdownlint-cli2 (0.17.2)
23-23: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
24-24: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
25-27: Format “Signing” section consistently
Update to:* **Signing:** * `signTypedData()`: Request the wallet to sign typed data according to EIP-712 (Starknet variant).🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
26-26: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
76-78: Consider adding a status badge
Enhance visibility by displaying build/test status or package version badge (e.g., from GitHub Actions or pub.dev) at the top of the README.
84-92: Refine hyphenation and list markers in “Type Safety”
- Remove hyphen in “strongly-typed” (adverbs ending in “-ly” need not be hyphenated).
- Normalize bullet items with one space after
*and two-space indentation:* Typed Models: ... * Enums: ... * Typed Methods: ... * Controlled `dynamic`: ...🧰 Tools
🪛 LanguageTool
[uncategorized] ~88-~88: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...heWalletProvidermethods have clear, strongly-typed signatures for both input parameters an...(HYPHENATED_LY_ADVERB_ADJECTIVE)
96-98: Align bullets under “Interaction with Standard Starknet JSON-RPC”
Use consistent formatting:* Standard JSON-RPC API: Primarily used by … * Wallet API (this provider): Primarily used by …
104-108: Standardize “Error Handling” list formatting
Refactor to:* Network & RPC Errors: … * Wallet-Specific Errors: … * Type Errors: … * Dapp Responsibility: …
113-118: Normalize “Future Implementation Recommendations” bullets
Ensure consistent two-space indent and a single space after*:* Wallet Connection/Discovery: … * Wallet Event Handling: … * Enhanced Testing Strategies: … * Higher-Level Abstractions (Potential): …
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (50)
packages/wallet_provider/.gitignore(1 hunks)packages/wallet_provider/.tool-versions(1 hunks)packages/wallet_provider/CHANGELOG.md(1 hunks)packages/wallet_provider/Implementation_notes.md(1 hunks)packages/wallet_provider/LICENSE(1 hunks)packages/wallet_provider/README.md(1 hunks)packages/wallet_provider/analysis_options.yaml(1 hunks)packages/wallet_provider/build.yaml(1 hunks)packages/wallet_provider/dart_test.yaml(1 hunks)packages/wallet_provider/example/wallet_provider_example.dart(1 hunks)packages/wallet_provider/lib/src/index.dart(1 hunks)packages/wallet_provider/lib/src/model/account_deployment_data.dart(1 hunks)packages/wallet_provider/lib/src/model/account_deployment_data.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/account_deployment_data.g.dart(1 hunks)packages/wallet_provider/lib/src/model/add_declare_transaction_result.dart(1 hunks)packages/wallet_provider/lib/src/model/add_declare_transaction_result.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/add_declare_transaction_result.g.dart(1 hunks)packages/wallet_provider/lib/src/model/add_invoke_transaction_result.dart(1 hunks)packages/wallet_provider/lib/src/model/add_invoke_transaction_result.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/add_invoke_transaction_result.g.dart(1 hunks)packages/wallet_provider/lib/src/model/api_version.dart(1 hunks)packages/wallet_provider/lib/src/model/asset.dart(1 hunks)packages/wallet_provider/lib/src/model/asset.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/asset.g.dart(1 hunks)packages/wallet_provider/lib/src/model/contract_class.dart(1 hunks)packages/wallet_provider/lib/src/model/contract_class.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/contract_class.g.dart(1 hunks)packages/wallet_provider/lib/src/model/declare_txn_wallet.dart(1 hunks)packages/wallet_provider/lib/src/model/declare_txn_wallet.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/declare_txn_wallet.g.dart(1 hunks)packages/wallet_provider/lib/src/model/deployment_version.dart(1 hunks)packages/wallet_provider/lib/src/model/index.dart(1 hunks)packages/wallet_provider/lib/src/model/invoke_call.dart(1 hunks)packages/wallet_provider/lib/src/model/invoke_call.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/invoke_call.g.dart(1 hunks)packages/wallet_provider/lib/src/model/permission.dart(1 hunks)packages/wallet_provider/lib/src/model/starknet_chain.dart(1 hunks)packages/wallet_provider/lib/src/model/starknet_chain.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/starknet_chain.g.dart(1 hunks)packages/wallet_provider/lib/src/model/typed_data.dart(1 hunks)packages/wallet_provider/lib/src/model/typed_data.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/typed_data.g.dart(1 hunks)packages/wallet_provider/lib/src/model/wallet_error.dart(1 hunks)packages/wallet_provider/lib/src/model/wallet_error.freezed.dart(1 hunks)packages/wallet_provider/lib/src/model/wallet_error.g.dart(1 hunks)packages/wallet_provider/lib/src/provider.dart(1 hunks)packages/wallet_provider/lib/wallet_provider.dart(1 hunks)packages/wallet_provider/pubspec.yaml(1 hunks)packages/wallet_provider/test/melos_test.dart(1 hunks)packages/wallet_provider/test/provider_test.dart(1 hunks)
✅ Files skipped from review due to trivial changes (7)
- packages/wallet_provider/lib/src/model/add_declare_transaction_result.g.dart
- packages/wallet_provider/lib/src/model/deployment_version.dart
- packages/wallet_provider/lib/src/index.dart
- packages/wallet_provider/lib/src/model/add_invoke_transaction_result.g.dart
- packages/wallet_provider/lib/src/model/typed_data.dart
- packages/wallet_provider/lib/src/model/contract_class.g.dart
- packages/wallet_provider/lib/src/model/account_deployment_data.g.dart
🚧 Files skipped from review as they are similar to previous changes (42)
- packages/wallet_provider/CHANGELOG.md
- packages/wallet_provider/dart_test.yaml
- packages/wallet_provider/.tool-versions
- packages/wallet_provider/lib/wallet_provider.dart
- packages/wallet_provider/LICENSE
- packages/wallet_provider/lib/src/model/permission.dart
- packages/wallet_provider/lib/src/model/api_version.dart
- packages/wallet_provider/build.yaml
- packages/wallet_provider/lib/src/model/index.dart
- packages/wallet_provider/lib/src/model/add_invoke_transaction_result.dart
- packages/wallet_provider/lib/src/model/wallet_error.g.dart
- packages/wallet_provider/lib/src/model/declare_txn_wallet.dart
- packages/wallet_provider/analysis_options.yaml
- packages/wallet_provider/lib/src/model/typed_data.g.dart
- packages/wallet_provider/test/melos_test.dart
- packages/wallet_provider/lib/src/model/declare_txn_wallet.g.dart
- packages/wallet_provider/lib/src/model/add_declare_transaction_result.dart
- packages/wallet_provider/pubspec.yaml
- packages/wallet_provider/.gitignore
- packages/wallet_provider/lib/src/model/invoke_call.g.dart
- packages/wallet_provider/lib/src/model/invoke_call.dart
- packages/wallet_provider/lib/src/model/asset.g.dart
- packages/wallet_provider/lib/src/model/asset.dart
- packages/wallet_provider/lib/src/model/account_deployment_data.dart
- packages/wallet_provider/lib/src/model/account_deployment_data.freezed.dart
- packages/wallet_provider/example/wallet_provider_example.dart
- packages/wallet_provider/lib/src/model/starknet_chain.dart
- packages/wallet_provider/Implementation_notes.md
- packages/wallet_provider/lib/src/model/starknet_chain.g.dart
- packages/wallet_provider/lib/src/model/wallet_error.dart
- packages/wallet_provider/lib/src/model/contract_class.dart
- packages/wallet_provider/lib/src/model/declare_txn_wallet.freezed.dart
- packages/wallet_provider/test/provider_test.dart
- packages/wallet_provider/lib/src/model/contract_class.freezed.dart
- packages/wallet_provider/lib/src/model/wallet_error.freezed.dart
- packages/wallet_provider/lib/src/model/add_invoke_transaction_result.freezed.dart
- packages/wallet_provider/lib/src/model/invoke_call.freezed.dart
- packages/wallet_provider/lib/src/model/asset.freezed.dart
- packages/wallet_provider/lib/src/model/starknet_chain.freezed.dart
- packages/wallet_provider/lib/src/model/typed_data.freezed.dart
- packages/wallet_provider/lib/src/model/add_declare_transaction_result.freezed.dart
- packages/wallet_provider/lib/src/provider.dart
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet_provider/lib/src/model/json_rpc_api_error.freezed.dart:26-31
Timestamp: 2024-11-21T18:01:50.217Z
Learning: The file `packages/starknet_provider/lib/src/model/json_rpc_api_error.freezed.dart` is auto-generated and should not be manually modified. Documentation changes should be made in the source file used to generate this code to ensure they persist upon regeneration.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/invoke_transaction.freezed.dart:26-31
Timestamp: 2024-10-26T15:39:54.600Z
Learning: In the `starknet_provider/lib/src/model/invoke_transaction.freezed.dart` file and other files with the `.freezed.dart` extension, the code is automatically generated by the `freezed` package and should not be manually modified.
🪛 LanguageTool
packages/wallet_provider/README.md
[uncategorized] ~13-~13: Loose punctuation mark.
Context: ...API versions. * supportedSpecs(): Get supported Starknet JSON-RPC spec ve...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...pec versions. * getPermissions(): Get existing permissions for the Dapp. ...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~15-~15: Loose punctuation mark.
Context: ...or the Dapp. * requestAccounts(): Request wallet account addresses. *...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~16-~16: Loose punctuation mark.
Context: ...nt addresses. * requestChainId(): Request the current chain ID from the w...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~19-~19: Loose punctuation mark.
Context: ...account. * switchStarknetChain(): Request to switch the wallet's active S...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~20-~20: Loose punctuation mark.
Context: ...et network. * addStarknetChain(): Request to add a new Starknet network c...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~21-~21: Loose punctuation mark.
Context: ...on to the wallet. * watchAsset(): Request to add an ERC20 token to the wa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~24-~24: Loose punctuation mark.
Context: ...ction. * addDeclareTransaction(): Request the wallet to sign and submit a...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~88-~88: Although a hyphen is possible, it is not necessary in a compound modifier in which the first word is an adverb that ends in ‘ly’.
Context: ...he WalletProvider methods have clear, strongly-typed signatures for both input parameters an...
(HYPHENATED_LY_ADVERB_ADJECTIVE)
🪛 markdownlint-cli2 (0.17.2)
packages/wallet_provider/README.md
3-3: Bare URL used
null
(MD034, no-bare-urls)
12-12: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
13-13: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
18-18: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
19-19: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
20-20: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
21-21: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
23-23: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
24-24: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
26-26: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🔇 Additional comments (9)
packages/wallet_provider/README.md (9)
5-6: The package description is clear and concise.
7-10: The “Features” heading and introductory line are well-structured.
28-31: The “Usage” intro lines effectively lead into the example.
32-72: The Dart example is comprehensive and illustrative; no markup issues detected within the fenced code block.
73-74: The advisory note is clear and informative.
80-83: The “Technical Considerations” and “Type Safety” headings are appropriately placed.
93-95: The “Interaction with Standard Starknet JSON-RPC” heading clearly sets context.
100-101: The “Error Handling” heading is concise.
109-111: The “Future Implementation Recommendations” heading introduces upcoming enhancements effectively.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/wallet_provider/lib/src/model/typed_data.dart (1)
25-36: Prioritize implementing a proper type-safe structure for the message fieldThe
messagefield is still usingMap<String, dynamic>, which reduces type safety for blockchain operations and could lead to subtle bugs.Consider implementing this as a proper class with defined fields based on the Starknet specification rather than using a dynamic map. Alternatively, you could create a generic type parameter that enforces proper structure:
- required Map<String, dynamic> message, + required Map<String, Object> message, // At minimum, avoid 'dynamic'For a more type-safe approach, consider creating a generic type parameter:
@freezed class TypedData<T> with _$TypedData<T> { const factory TypedData({ required Map<String, List<StarknetTypeDescriptor>> types, required String primaryType, required StarknetDomain domain, required T message, }) = _TypedData<T>; // JSON conversion would need to be adjusted }
🧹 Nitpick comments (1)
packages/wallet_provider/lib/src/model/typed_data.dart (1)
23-36: Consider adding utility constructors for common TypedData patternsCreating
TypedDataobjects can be complex due to the nested structure. Adding utility constructors would improve developer experience.// Example utility constructor factory TypedData.simple({ required String primaryType, required List<StarknetTypeDescriptor> typeDescriptors, required StarknetDomain domain, required Map<String, Object> message, }) { return TypedData( types: {primaryType: typeDescriptors}, primaryType: primaryType, domain: domain, message: message, ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
packages/wallet_provider/lib/src/model/asset.dart(1 hunks)packages/wallet_provider/lib/src/model/contract_class.dart(1 hunks)packages/wallet_provider/lib/src/model/typed_data.dart(1 hunks)packages/wallet_provider/lib/src/provider.dart(1 hunks)packages/wallet_provider/lib/wallet_provider.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/wallet_provider/lib/wallet_provider.dart
- packages/wallet_provider/lib/src/model/asset.dart
- packages/wallet_provider/lib/src/model/contract_class.dart
- packages/wallet_provider/lib/src/provider.dart
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet_provider/lib/src/model/json_rpc_api_error.freezed.dart:26-31
Timestamp: 2024-11-21T18:01:50.217Z
Learning: The file `packages/starknet_provider/lib/src/model/json_rpc_api_error.freezed.dart` is auto-generated and should not be manually modified. Documentation changes should be made in the source file used to generate this code to ensure they persist upon regeneration.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/invoke_transaction.freezed.dart:26-31
Timestamp: 2024-10-26T15:39:54.600Z
Learning: In the `starknet_provider/lib/src/model/invoke_transaction.freezed.dart` file and other files with the `.freezed.dart` extension, the code is automatically generated by the `freezed` package and should not be manually modified.
packages/wallet_provider/lib/src/model/typed_data.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/invoke_transaction.freezed.dart:26-31
Timestamp: 2024-10-26T15:39:54.600Z
Learning: In the `starknet_provider/lib/src/model/invoke_transaction.freezed.dart` file and other files with the `.freezed.dart` extension, the code is automatically generated by the `freezed` package and should not be manually modified.
🪛 GitHub Actions: CI
packages/wallet_provider/lib/src/model/typed_data.dart
[error] 10-10: Type '_$StarknetTypeDescriptor' not found. The class StarknetTypeDescriptor with _$StarknetTypeDescriptor mixin cannot be resolved.
[error] 10-10: The type '_$StarknetTypeDescriptor' can't be mixed in.
[error] 14-14: Couldn't find constructor '_StarknetTypeDescriptor'. Redirection constructor target not found.
[error] 17-17: Method not found: '_$StarknetTypeDescriptorFromJson'.
🪛 GitHub Actions: code_health
packages/wallet_provider/lib/src/model/typed_data.dart
[error] 10-10: Dart analyze error: Classes can only mix in mixins and classes. (mixin_of_non_class)
[error] 14-14: Dart analyze error: The name '_StarknetTypeDescriptor' isn't a type and can't be used in a redirected constructor. Possibly a typo or missing type definition. (non_type_as_type_argument)
🔇 Additional comments (1)
packages/wallet_provider/lib/src/model/typed_data.dart (1)
47-49: Great job implementing domain validationYou've successfully addressed the previous review comment by adding validation to ensure at least one field in the
StarknetDomainis non-null.
| import 'package:freezed_annotation/freezed_annotation.dart'; | ||
| import 'package:starknet/starknet.dart'; // Import Felt | ||
|
|
||
| part 'typed_data.freezed.dart'; | ||
| part 'typed_data.g.dart'; | ||
|
|
There was a problem hiding this comment.
Run code generation for Freezed models
The CI pipeline is failing because the generated files referenced in the part directives don't exist or are outdated. You need to run the code generation command after making changes to this file.
Execute the following command in your terminal from the project root:
dart run build_runner build --delete-conflicting-outputs| const factory TypedData({ | ||
| /// Defines the custom types used in the message. Maps struct names to their field descriptors. | ||
| /// Example: { "Mail": [ { "name": "from", "type": "felt" }, ... ], "Person": [...] } | ||
| required Map<String, List<StarknetTypeDescriptor>> types, | ||
| /// The primary struct type to hash and sign (e.g., "Mail"). Must exist as a key in `types`. | ||
| required String primaryType, | ||
| /// Domain separator parameters to ensure signature uniqueness across different applications/chains. |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for primaryType to ensure it exists in types map
The primaryType field must exist as a key in the types map, but this constraint isn't enforced in the code.
Add an assertion to validate this relationship:
@freezed
class TypedData with _$TypedData {
+ @Assert('types.containsKey(primaryType)', 'primaryType must exist in types map')
const factory TypedData({
/// Defines the custom types used in the message. Maps struct names to their field descriptors.
/// Example: { "Mail": [ { "name": "from", "type": "felt" }, ... ], "Person": [...] }
required Map<String, List<StarknetTypeDescriptor>> types,
/// The primary struct type to hash and sign (e.g., "Mail"). Must exist as a key in `types`.
required String primaryType,📝 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.
| const factory TypedData({ | |
| /// Defines the custom types used in the message. Maps struct names to their field descriptors. | |
| /// Example: { "Mail": [ { "name": "from", "type": "felt" }, ... ], "Person": [...] } | |
| required Map<String, List<StarknetTypeDescriptor>> types, | |
| /// The primary struct type to hash and sign (e.g., "Mail"). Must exist as a key in `types`. | |
| required String primaryType, | |
| /// Domain separator parameters to ensure signature uniqueness across different applications/chains. | |
| @Assert('types.containsKey(primaryType)', 'primaryType must exist in types map') | |
| const factory TypedData({ | |
| /// Defines the custom types used in the message. Maps struct names to their field descriptors. | |
| /// Example: { "Mail": [ { "name": "from", "type": "felt" }, ... ], "Person": [...] } | |
| required Map<String, List<StarknetTypeDescriptor>> types, | |
| /// The primary struct type to hash and sign (e.g., "Mail"). Must exist as a key in `types`. | |
| required String primaryType, | |
| /// Domain separator parameters to ensure signature uniqueness across different applications/chains. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/wallet_provider/lib/src/provider.dart (3)
106-112: Avoid using print statements in library codeUsing
Consider implementing a more flexible logging approach:
- try { - return Permission.values.byName(p); - } catch (e) { - // Handle cases where the permission string is not in the enum - print('Warning: Unknown permission received: $p'); - return null; // Or throw an error, depending on desired strictness - } + try { + return Permission.values.byName(p); + } catch (e) { + // Handle cases where the permission string is not in the enum + // Silently return null to be filtered out later + // Consider exposing an optional logger or callback for unknown permissions + return null; + }
155-173: Complex positional parameter handling in switchStarknetChainThe logic for handling positional parameters with potential nulls is complex but correct. Consider adding more documentation explaining this pattern.
Add more documentation explaining why positional parameters are handled differently in this method compared to others:
/// Change the current network of the wallet. +/// +/// Note: This method uses positional parameters according to the Wallet API spec: +/// [chainId, silent_mode?, api_version?] Future<bool> switchStarknetChain(Felt chainId, {bool? silentMode, ApiVersion? apiVersion}) async { // Sticking to pure positional: [chainId, silent_mode?, api_version?]
80-235: Consider defensive type checking for JSON-RPC responsesWhile you do cast results to expected types, consider adding more defensive checks before these casts to handle unexpected response formats gracefully.
For example, in a method like
signTypedData, you could add a check:final result = await _client.call('wallet_signTypedData', params); // Result is SIGNATURE (List<String> or List<dynamic>) +if (result is! List) { + throw FormatException('Expected array signature result, got: ${result.runtimeType}'); +} final signature = List<String>.from(result as List); return signature.map((s) => Felt.fromHexString(s)).toList();This approach would provide clearer error messages if the wallet implementation returns unexpected data structures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/wallet_provider/lib/src/model/asset.dart(1 hunks)packages/wallet_provider/lib/src/model/typed_data.dart(1 hunks)packages/wallet_provider/lib/src/provider.dart(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/wallet_provider/lib/src/model/asset.dart
- packages/wallet_provider/lib/src/model/typed_data.dart
🔇 Additional comments (7)
packages/wallet_provider/lib/src/provider.dart (7)
1-5: Correct import organization with explicit TypedData hidingThe imports are well-organized, with explicit hiding of the
TypedDatafromstarknet.dartto avoid conflicts with your own implementation. This demonstrates good awareness of potential name collisions.
8-22: Well-implemented JsonRpcClient with monotonically increasing IDsThe implementation follows JSON-RPC 2.0 specification and incorporates previous review feedback by using a monotonically increasing request ID counter instead of hard-coded values. This prevents potential response mismatches during concurrent requests.
24-31: Good HTTP headers implementation with Accept headerYour code now includes both proper Content-Type and Accept headers as recommended in previous reviews, which follows HTTP best practices for JSON-RPC requests.
33-73: Robust error handling with multiple validation layersYour implementation properly handles different error scenarios:
- JSON parsing errors
- JSON-RPC protocol errors
- HTTP status code errors
- Malformed responses
The structured approach to error handling will provide clear, actionable error messages to developers.
118-137: Thorough parameter handling for requestAccountsThe implementation carefully handles the optional parameters for
requestAccountsaccording to the specification, conditionally constructing the parameter object only when needed. The conversion from hex strings toFeltobjects is also properly implemented.
219-229: Consistent TypedData signing implementationThe signing implementation properly converts the typed data to JSON and transforms the response signature from hex strings to
Feltvalues, maintaining consistency with other wallet operations.
1-235: Overall excellent Wallet API provider implementationThe implementation is well-structured, thoroughly documented, and properly implements the Starknet Wallet API JSON-RPC specification. The code demonstrates good error handling, proper type conversion, and attention to edge cases. The use of typed methods with clear parameters and return types makes this package easy to use for Dart developers working with Starknet wallets.
|
i did some updates from coderabbit recommandations |
This PR suggests to manage issue #490
Notes on Starknet Wallet API Implementation for starknet.dart
1. Objective
The goal was to study the Starknet Wallet API specification (based on
wallet_rpc.json) and propose an implementation as a distinct Dart package (wallet_provider), usable within thestarknet.dartecosystem. This package should allow Dart applications to interact with a compatible Starknet wallet.2. Implementation Approach
wallet_provider, was created by duplicating then modifyingstarknet_providerto isolate the logic specific to the Wallet API.wallet_rpc.jsonwere implemented in Dart using thefreezedpackage to generate immutable and serializable classes (files inlib/src/model/).WalletProviderclass (lib/src/provider.dart) was created. It exposes typed methods corresponding to the Wallet API RPC calls (wallet_supportedWalletApi,wallet_addInvokeTransaction, etc.).JsonRpcClientwas implemented inprovider.dartto handle sending JSON-RPC POST requests and basic deserialization of responses and errors.package:starknetfor fundamental types (Felt) and some shared complex types (EntryPointsByType). It also usespackage:httpfor network calls.wallet_providerpackage to focus it solely on the Wallet API.3. Discoveries and Key Points
wallet_rpc.json) is structured enough to allow for the generation (here, manually assisted) of type-safe Dart data models usingfreezed.package:starknet(Felt,EntryPointsByType, etc.). Version compatibility and the clarity of exports from this package are crucial.paramsin the JSON-RPC request) is not always explicitly defined in the spec as being a positional list or a single object. The current implementation makes assumptions based on the order and apparent structure inwallet_rpc.json, but this might require adjustments based on actual wallet implementations.signTypedDataComplexity: Signing typed data requires careful construction of theTypedDataobject, especially converting all message and domain values into their correspondingFelttype (short string encoding, integer/hex conversion, etc.) to match the types defined in thetypesstructure.4. Challenges Encountered
ContractClass): There was initial confusion about the origin and correct definition ofContractClass, requiring determination of whether to use a local definition (from the oldstarknet_provider) or the one from thestarknetpackage. Ultimately, a local definition based on the schema was created.5. Recommendations
paramsfor each method (positional list vs. object).TypedDataUtilities: Consider adding utility functions (potentially inpackage:starknetorwallet_provider) to facilitate the correct construction ofTypedDataobjects, especially for String -> Felt conversion according to EIP-712/Starknet rules.Summary by CodeRabbit
New Features
wallet_providerDart package for interacting with Starknet wallets via the Wallet API.Documentation
Tests
Chores