feat: Add support for declare, deploy, and invoke v3. Also add support for estimate_fee #423
Conversation
…for estimate_fee. All tested in devnet 0.1.2 with "melos test".
|
To view this pull requests documentation preview, visit the following URL: docs.page/focustree/starknet.dart~423 Documentation is deployed and generated using docs.page. |
WalkthroughThis pull request introduces comprehensive updates across the StarkNet Dart package, focusing on version upgrades, transaction model enhancements, and improved fee estimation capabilities. The changes span multiple files and involve updating dependency versions, introducing new transaction versions (v3), expanding serialization methods, and refining contract interaction models. Key modifications include updating StarkNet devnet versions, adding support for more complex transaction types, and enhancing cryptographic and fee estimation functionalities. Changes
Possibly Related PRs
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🔭 Outside diff range comments (1)
packages/starknet/lib/src/account.dart (1)
Line range hint
437-545: Handle transaction submission errors gracefullyIn the
executemethod, ensure that all possible exceptions are caught and handled appropriately, especially when dealing with network failures or invalid responses.Consider wrapping the transaction submission in a try-catch block to handle unforeseen errors.
🧹 Nitpick comments (22)
contracts/v2.6.2/Scarb.toml (1)
2-2: Consider using a more descriptive package nameThe name "contract2" appears to be a temporary or generic name. Consider using a more meaningful name that reflects the package's purpose, perhaps following the original naming convention (e.g., "starknet_dart_v3" or similar).
packages/starknet/lib/src/contract/model/compiled_contract.freezed.dart (4)
34-34: Simplify@JsonKeyAnnotation Usingignore: trueInstead of using
@JsonKey(includeFromJson: false, includeToJson: false)on thecopyWithmethod, you can use@JsonKey(ignore: true)for brevity and clarity, as it effectively achieves the same result.Apply this diff to simplify the annotation:
- @JsonKey(includeFromJson: false, includeToJson: false) + @JsonKey(ignore: true)
220-221: Simplify@JsonKeyAnnotation onhashCodeMethodSimilarly, for the
hashCodemethod in_$SierraCompiledContractImpl, you can simplify the annotation:Apply this diff:
- @JsonKey(includeFromJson: false, includeToJson: false) + @JsonKey(ignore: true)
229-231: Simplify@JsonKeyAnnotation oncopyWithGetterFor consistency, consider simplifying the
@JsonKeyannotation on thecopyWithgetter as well.Apply this diff:
- @JsonKey(includeFromJson: false, includeToJson: false) + @JsonKey(ignore: true)
799-799: Simplify@JsonKeyAnnotation oncopyWithGetterAs with previous instances, consider simplifying the
@JsonKeyannotation on thecopyWithgetter inDeprecatedCompiledContract.Apply this diff:
- @JsonKey(includeFromJson: false, includeToJson: false) + @JsonKey(ignore: true)contracts/v2.6.2/src/erc20.cairo (1)
35-36: Consider making token name and symbol constructor parametersTo enhance the reusability and flexibility of the contract, it's advisable to accept
nameandsymbolas parameters to the constructor instead of hardcoding them. This allows for the deployment of multiple instances with different token names and symbols without modifying the contract code.Apply this diff to implement the change:
#[constructor] fn constructor( ref self: ContractState, initial_supply: u256, recipient: ContractAddress + , + name: felt252, + symbol: felt252 ) { - let name = "MyToken"; - let symbol = "MTK"; self.erc20.initializer(name, symbol); self.erc20._mint(recipient, initial_supply); }packages/starknet/lib/src/contract/contract.dart (1)
52-66: Add documentation for new parameters inexecutemethodThe
executemethod now includes several new optional parameters. To improve code readability and usability, consider adding documentation comments for each parameter to explain their purpose and usage.packages/starknet_provider/lib/src/model/invoke_transaction.dart (1)
40-41: Include unsupported version in error messageIncluding the unsupported version in the error message will aid in debugging and provide clearer feedback.
Apply this diff:
default: - throw ArgumentError('Unsupported transaction version:'); + throw ArgumentError('Unsupported transaction version: ${json['version']}');packages/starknet/lib/src/contract/model/compiled_class_hash.dart (1)
28-29: Simplify redundant mapping in hash computationsThe expressions
data.map((e) => e).toList()andhashInputs.map((e) => e).toList()can be simplified todataandhashInputsrespectively, as they are mapping each element to itself.Apply this diff to simplify the code:
@@ -28,2 +28,2 @@ - BigInt hash() { - return (poseidonHasher.hashMany(data.map((e) => e).toList())); + BigInt hash() { + return poseidonHasher.hashMany(data); @@ -49,2 +49,2 @@ - return (poseidonHasher.hashMany(hashInputs.map((e) => e).toList())) + - BigInt.from(1); + return poseidonHasher.hashMany(hashInputs) + + BigInt.from(1);Also applies to: 49-50
packages/starknet/lib/src/signer.dart (2)
84-86: ReplaceThe
Consider using a logger:
// Replace print statements with logging logger.info( 'signature: ${Felt(signature.r).toHexString()} ${Felt(signature.s).toHexString()}');
416-425: ReplaceSeveral
classHash,calldata,salt, and contract address. In production code, it's advisable to use a logging framework instead ofConsider implementing a logger and replacing
logger.info('classHash: ${classHash.toHexString()}'); logger.info('calldata: ${constructorCalldata.map((e) => e.toHexString())}'); logger.info('salt: ${contractAddressSalt.toHexString()}'); logger.info('[signDeployAccountTransactionV3] Contract address: ${contractAddress.toHexString()}');packages/starknet/test/account_test.dart (1)
271-274: Unused result in balance retrievalThe result of the balance retrieval is computed but not used. Consider assigning the value to a variable or removing the unnecessary computation.
- result.when( - result: (result) => result[0].toInt(), - error: (error) => throw Exception("Failed to get balance"), - );If the balance value is needed later, store it in a variable:
final balance = result.when( result: (result) => result[0].toInt(), error: (error) => throw Exception("Failed to get balance"), );packages/starknet_provider/test/integration/read_provider_test.dart (1)
1052-1074: Add checks for unit consistency in V3 fee estimation testIn the fee estimation test for the V3 Invoke transaction, consider verifying that the units of gas price and data gas price are consistent and match expected values.
expect(estimate.gasPriceUnit, equals('WEI')); expect(estimate.dataGasPriceUnit, equals('WEI'));packages/starknet/lib/src/account.dart (4)
4-4: Organize importsConsider grouping the
dart:ioanddart:mathimports together for better organization.import 'dart:io'; +import 'dart:math';
68-165: Refactor fee estimation methods to reduce duplicationThe methods
getEstimateMaxFeeForInvokeTx,getEstimateMaxFeeForDeclareTx, andgetEstimateMaxFeeForDeployAccountTxhave similar structures and logic. Consider refactoring common code into a shared helper method to improve maintainability.
289-353: Ensure consistency in parameter naming and default valuesIn
getEstimateMaxFeeForDeployAccountTx, verify that the parameter names and default values are consistent with other methods to prevent confusion.
356-396: Missing documentation for new methodThe method
getEstimateMaxFeeForDeployTxlacks documentation. Adding a docstring will help other developers understand its purpose and usage.packages/starknet_provider/lib/src/model/deploy_account_transaction.freezed.dart (1)
254-254: Remove commented code or clarify necessityThere is a commented line
//Use 0x1 instead of 0x01 for devnet compatibility. Consider removing commented code or adding clarification if necessary.packages/starknet/lib/src/presets/udc.g.dart (1)
17-27: Consider adding documentation for new parameters.The new parameters support v3 transactions, but their purpose and usage might not be immediately clear to users. Consider adding documentation comments to explain:
- The purpose of each fee-related parameter
- The relationship between maxFee and the L1/L2 fee parameters
- The meaning of feeDataAvailabilityMode and nonceDataAvailabilityMode
contracts/v2.6.2/src/counter.cairo (2)
62-66: Fix typos in test commentsSeveral comments contain spelling errors:
- Line 62: "deplyment" should be "deployment", "settonh" should be "setting"
- Line 65: "getting the" has extra space
- Line 72: "conter" should be "counter"
- /// doing deplyment of the contract settonh the contractor at one + /// Deploying the contract and setting the counter at one - /// getting the contract count + /// Getting the contract count - /// testing if the conter is incrementing as expected + /// Testing if the counter is incrementing as expectedAlso applies to: 68-74, 76-78
82-83: Consider moving deployment addresses to configurationThe hardcoded class hash and contract address should be moved to a configuration file or environment variables for better maintainability.
contracts/v1/artifacts/contract2_Counter.compiled_contract_class.json (1)
1-1: Reminder: Avoid manual modifications to compiled artifacts.This file is automatically generated by the Cairo compiler. Any manual changes to this file should be avoided as they could break contract functionality. Instead, modify the source Cairo files and recompile.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
contracts/v2.6.2/Scarb.lockis excluded by!**/*.lock
📒 Files selected for processing (44)
.env.devnet(1 hunks).tool-versions(1 hunks)contracts/v1/artifacts/contract2_Counter.compiled_contract_class.json(1 hunks)contracts/v1/artifacts/contract2_Counter.contract_class.json(1 hunks)contracts/v2.6.2/Scarb.toml(1 hunks)contracts/v2.6.2/src/counter.cairo(1 hunks)contracts/v2.6.2/src/erc20.cairo(1 hunks)contracts/v2.6.2/src/lib.cairo(1 hunks)packages/starknet/lib/src/account.dart(12 hunks)packages/starknet/lib/src/contract/contract.dart(3 hunks)packages/starknet/lib/src/contract/erc20.dart(3 hunks)packages/starknet/lib/src/contract/model/compiled_class_hash.dart(1 hunks)packages/starknet/lib/src/contract/model/compiled_contract.dart(3 hunks)packages/starknet/lib/src/contract/model/compiled_contract.freezed.dart(32 hunks)packages/starknet/lib/src/contract/model/compiled_contract.g.dart(2 hunks)packages/starknet/lib/src/contract/model/contract_abi.freezed.dart(96 hunks)packages/starknet/lib/src/contract/model/entry_points_by_type.freezed.dart(30 hunks)packages/starknet/lib/src/crypto/model/pedersen_params.freezed.dart(5 hunks)packages/starknet/lib/src/crypto/model/poseidon_params.freezed.dart(6 hunks)packages/starknet/lib/src/presets/udc.g.dart(2 hunks)packages/starknet/lib/src/signer.dart(7 hunks)packages/starknet/lib/src/static_config.dart(1 hunks)packages/starknet/lib/src/types/felt.dart(1 hunks)packages/starknet/test/account_test.dart(2 hunks)packages/starknet/test/model.freezed.dart(5 hunks)packages/starknet/test/types/felt_test.dart(1 hunks)packages/starknet_provider/.tool-versions(1 hunks)packages/starknet_provider/lib/src/model/declare_transaction.dart(5 hunks)packages/starknet_provider/lib/src/model/declare_transaction.freezed.dart(11 hunks)packages/starknet_provider/lib/src/model/declare_transaction.g.dart(3 hunks)packages/starknet_provider/lib/src/model/deploy_account_transaction.dart(2 hunks)packages/starknet_provider/lib/src/model/deploy_account_transaction.freezed.dart(5 hunks)packages/starknet_provider/lib/src/model/deploy_account_transaction.g.dart(2 hunks)packages/starknet_provider/lib/src/model/estimate_fee.dart(5 hunks)packages/starknet_provider/lib/src/model/estimate_fee.g.dart(3 hunks)packages/starknet_provider/lib/src/model/fee_estimate.dart(1 hunks)packages/starknet_provider/lib/src/model/fee_estimate.freezed.dart(8 hunks)packages/starknet_provider/lib/src/model/fee_estimate.g.dart(1 hunks)packages/starknet_provider/lib/src/model/get_transaction.dart(2 hunks)packages/starknet_provider/lib/src/model/get_transaction.freezed.dart(4 hunks)packages/starknet_provider/lib/src/model/invoke_transaction.dart(4 hunks)packages/starknet_provider/lib/src/model/invoke_transaction.freezed.dart(3 hunks)packages/starknet_provider/lib/src/model/invoke_transaction.g.dart(3 hunks)packages/starknet_provider/test/integration/read_provider_test.dart(3 hunks)
✅ Files skipped from review due to trivial changes (6)
- .tool-versions
- packages/starknet_provider/.tool-versions
- .env.devnet
- packages/starknet_provider/lib/src/model/get_transaction.dart
- contracts/v2.6.2/src/lib.cairo
- contracts/v1/artifacts/contract2_Counter.contract_class.json
🧰 Additional context used
📓 Learnings (15)
packages/starknet/test/types/felt_test.dart (5)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/test/types/felt_test.dart:12-29
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `packages/starknet/test/types/felt_test.dart`, for the `Felt` class operator tests, the current test cases are sufficient, and additional error condition tests are not necessary.
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet/lib/src/types/felt.dart:24-31
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the `Felt` class in `packages/starknet/lib/src/types/felt.dart`, when implementing the `>>` operator, we should include a validation check to ensure that the result does not exceed `prime`, throwing an error if it does.
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet/lib/src/types/felt.dart:42-48
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `packages/starknet/lib/src/types/felt.dart`, in the `Felt` class, the subtraction operator `operator -(Felt other)` should throw an error if the result is negative, rather than performing modular subtraction.
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet/lib/src/types/felt.dart:15-22
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the `Felt` class (`packages/starknet/lib/src/types/felt.dart`), when implementing operators like `<<` and `>>`, if the result exceeds `prime`, an error should be thrown instead of performing modular reduction.
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet/lib/src/types/felt.dart:50-54
Timestamp: 2024-11-21T20:50:57.873Z
Learning: In the `Felt` class (`packages/starknet/lib/src/types/felt.dart`), the multiplication operator does not require a prime modulo check, as the `Felt` class always ensures that the field range is checked.
packages/starknet_provider/test/integration/read_provider_test.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet_provider/lib/src/model/estimate_fee.dart:103-122
Timestamp: 2024-11-21T18:27:07.722Z
Learning: In the file `packages/starknet_provider/lib/src/model/estimate_fee.dart`, for version 3 invoke transactions (`BroadcastedInvokeTxnV3`), the `maxFee` field is not required.
packages/starknet_provider/lib/src/model/invoke_transaction.g.dart (2)
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet_provider/lib/src/model/invoke_transaction.g.dart:27-27
Timestamp: 2024-11-21T18:02:58.562Z
Learning: In the `packages/starknet_provider` project, it's acceptable to include changes to auto-generated `.g.dart` files like `packages/starknet_provider/lib/src/model/invoke_transaction.g.dart` in commits, as they are necessary for the functionality.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/invoke_transaction.g.dart:94-97
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In this project, files with the .g.dart extension are automatically generated and should not be modified manually.
packages/starknet/lib/src/contract/model/compiled_contract.dart (3)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/lib/src/contract/model/compiled_class_hash.dart:43-51
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the file `packages/starknet/lib/src/contract/model/compiled_class_hash.dart`, the formula used in the `hash` method of `BytecodeSegmentedNode` comes from an official implementation and should not be changed.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/lib/src/contract/model/compiled_class_hash.dart:27-28
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the file `packages/starknet/lib/src/contract/model/compiled_class_hash.dart`, the formula used in the `hash` method of `BytecodeLeaf` comes from the original source and should not be changed.
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet/lib/src/contract/model/compiled_class_hash.dart:132-135
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the file `packages/starknet/lib/src/contract/model/compiled_class_hash.dart`, the formula used in the `_createBytecodeSegmentStructureInner` function comes from an official implementation and should not be changed.
packages/starknet_provider/lib/src/model/declare_transaction.dart (7)
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet_provider/lib/src/model/declare_transaction.dart:0-0
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `packages/starknet_provider/lib/src/model/declare_transaction.dart`, the `max_fee` field in `DeclareTransactionV2` is of type `String` instead of `Felt` because the devnet only supports a 16-byte length for this field, so we can't send this field as a `Felt` (it's too long).
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet_provider/lib/src/model/declare_transaction.dart:64-65
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `DeclareTransactionV2`, the `max_fee` field is of type `String` because the target RPC host doesn't support the `Felt` data type due to its length.
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet_provider/lib/src/model/declare_transaction.g.dart:54-54
Timestamp: 2024-11-12T04:15:58.361Z
Learning: Files ending with `.g.dart` are autogenerated and should not be manually modified or reviewed for changes.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/declare_transaction.dart:91-91
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `declare_transaction.dart`, the `resourceBounds` map currently only supports 2 hardcoded types, but this may change in the future.
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet_provider/lib/src/model/estimate_fee.dart:156-176
Timestamp: 2024-11-21T18:27:17.609Z
Learning: In Dart's `estimate_fee.dart` file, the `BroadcastedDeclareTxnV3` class for version 3 declare transactions does not require a `maxFee` field, as `maxFee` is not required in version 3 declare transactions.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/declare_transaction.dart:103-106
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `packages/starknet_provider/lib/src/model/declare_transaction.dart`, within the `ResourceBounds` class, the fields `maxAmount` and `maxPricePerUnit` must remain as `String` type because they need to be sent as strings to the target host.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/invoke_transaction.dart:85-85
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `packages/starknet_provider/lib/src/model/invoke_transaction.dart`, the `ResourceBounds` class is accessible via the import of `package:starknet_provider/starknet_provider.dart`, so explicit import of `declare_transaction.dart` is unnecessary.
packages/starknet/test/account_test.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/test/account_test.dart:63-65
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the test files such as 'packages/starknet/test/account_test.dart', code duplication of contract artifact paths is acceptable, and refactoring suggestions to extract constants or helper methods are not required.
contracts/v1/artifacts/contract2_Counter.compiled_contract_class.json (3)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: contracts/v1/artifacts/contract2_Counter2.contract_class.json:1-1
Timestamp: 2024-11-12T04:15:58.361Z
Learning: Files in `contracts/v1/artifacts/` are automatically generated by the compiler and should not be modified directly.
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: contracts/v1/artifacts/contract2_Counter.compiled_contract_class.json:1-1
Timestamp: 2024-11-12T04:15:58.361Z
Learning: The file `contracts/v1/artifacts/contract2_Counter.compiled_contract_class.json` contains the correct Counter contract implementation and does not contain ERC20 functions.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: contracts/v1/artifacts/contract2_Counter3.contract_class.json:1-1
Timestamp: 2024-11-12T04:15:58.361Z
Learning: Files in the `contracts/v1/artifacts/` directory are automatically generated by the compiler and should not be modified manually.
packages/starknet_provider/lib/src/model/declare_transaction.g.dart (2)
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet_provider/lib/src/model/declare_transaction.g.dart:54-54
Timestamp: 2024-11-12T04:15:58.361Z
Learning: Files ending with `.g.dart` are autogenerated and should not be manually modified or reviewed for changes.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/declare_transaction.dart:91-91
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `declare_transaction.dart`, the `resourceBounds` map currently only supports 2 hardcoded types, but this may change in the future.
contracts/v2.6.2/src/counter.cairo (4)
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: contracts/v2.6.2/src/counter.cairo:44-80
Timestamp: 2024-11-12T04:15:58.361Z
Learning: The file `contracts/v2.6.2/src/counter.cairo` is just an example and does not require additional tests.
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: contracts/v2.6.2/src/counter.cairo:24-27
Timestamp: 2024-11-21T17:57:27.555Z
Learning: In the `contracts/v2.6.2/src/counter.cairo` file, which contains example code written in Cairo, it's acceptable to omit overflow protection in methods like `increment` and `increase_count_by`, as the file is intended for demonstration purposes only.
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: contracts/v2.6.2/src/counter.cairo:28-31
Timestamp: 2024-11-12T04:15:58.361Z
Learning: The file `contracts/v2.6.2/src/counter.cairo` is an example file and doesn't need additional checks.
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: contracts/v2.6.2/src/counter.cairo:24-27
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the `contracts/v2.6.2/src/counter.cairo` file, the code is an example and does not require additional checks.
packages/starknet_provider/lib/src/model/invoke_transaction.dart (3)
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet_provider/lib/src/model/invoke_transaction.g.dart:27-27
Timestamp: 2024-11-21T18:02:58.562Z
Learning: In the `packages/starknet_provider` project, it's acceptable to include changes to auto-generated `.g.dart` files like `packages/starknet_provider/lib/src/model/invoke_transaction.g.dart` in commits, as they are necessary for the functionality.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/invoke_transaction.g.dart:94-97
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In this project, files with the .g.dart extension are automatically generated and should not be modified manually.
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet_provider/lib/src/model/invoke_transaction.dart:83-104
Timestamp: 2024-11-21T23:08:45.593Z
Learning: In `packages/starknet_provider/lib/src/model/invoke_transaction.dart`, for the `InvokeTransactionV3` class in Dart, data availability modes `feeDataAvailabilityMode` and `nonceDataAvailabilityMode` are used as strings 'L1' and 'L2', and enums are not required at this point.
packages/starknet_provider/lib/src/model/estimate_fee.dart (4)
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet_provider/lib/src/model/estimate_fee.freezed.dart:167-167
Timestamp: 2024-11-12T04:15:58.361Z
Learning: Files generated by `Freezed` are autogenerated and should not be modified manually.
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet_provider/lib/src/model/estimate_fee.dart:156-176
Timestamp: 2024-11-21T18:27:17.609Z
Learning: In Dart's `estimate_fee.dart` file, the `BroadcastedDeclareTxnV3` class for version 3 declare transactions does not require a `maxFee` field, as `maxFee` is not required in version 3 declare transactions.
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet_provider/lib/src/model/estimate_fee.dart:103-122
Timestamp: 2024-11-21T18:27:07.722Z
Learning: In the file `packages/starknet_provider/lib/src/model/estimate_fee.dart`, for version 3 invoke transactions (`BroadcastedInvokeTxnV3`), the `maxFee` field is not required.
Learnt from: rukafe0x
PR: focustree/starknet.dart#417
File: packages/starknet_provider/lib/src/model/estimate_fee.dart:204-223
Timestamp: 2024-11-21T18:25:36.194Z
Learning: In v3 deploy account transactions (`BroadcastedDeployAccountTxnV3`), `maxFee` is not required.
packages/starknet_provider/lib/src/model/get_transaction.freezed.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-11-12T04:15:58.361Z
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/starknet/lib/src/contract/model/compiled_class_hash.dart (6)
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet/lib/src/contract/model/compiled_class_hash.dart:132-135
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the file `packages/starknet/lib/src/contract/model/compiled_class_hash.dart`, the formula used in the `_createBytecodeSegmentStructureInner` function comes from an official implementation and should not be changed.
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet/lib/src/contract/model/compiled_class_hash.dart:0-0
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the file `packages/starknet/lib/src/contract/model/compiled_class_hash.dart`, the `bytecode` list is validated by the originator, ensuring that `bytecodeOffset` and `segmentEnd` are within bounds in `_createBytecodeSegmentStructureInner`, so range errors during sublist extraction are not a concern.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/lib/src/contract/model/compiled_class_hash.dart:184-190
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the file `packages/starknet/lib/src/contract/model/compiled_class_hash.dart`, the structure and placement of classes, including `_BytecodeSegmentStructureResult`, come from the original source and should not be changed.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/lib/src/contract/model/compiled_class_hash.dart:43-51
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the file `packages/starknet/lib/src/contract/model/compiled_class_hash.dart`, the formula used in the `hash` method of `BytecodeSegmentedNode` comes from an official implementation and should not be changed.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/lib/src/contract/model/compiled_class_hash.dart:27-28
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the file `packages/starknet/lib/src/contract/model/compiled_class_hash.dart`, the formula used in the `hash` method of `BytecodeLeaf` comes from the original source and should not be changed.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/lib/src/contract/model/compiled_class_hash.dart:7-7
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the file `packages/starknet/lib/src/contract/model/compiled_class_hash.dart`, the `NestedIntList` typedef uses `dynamic` because it is an exact translation from the original source and should not be changed.
packages/starknet/lib/src/contract/model/compiled_contract.freezed.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/lib/src/contract/model/compiled_contract.dart:158-158
Timestamp: 2024-11-12T04:15:58.361Z
Learning: All contracts have `bytecodeSegmentLengths`, so the `bytecodeSegmentLengths` parameter in the `CASMCompiledContract` constructor can be made required.
packages/starknet_provider/lib/src/model/declare_transaction.freezed.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet_provider/lib/src/model/declare_transaction.dart:0-0
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `packages/starknet_provider/lib/src/model/declare_transaction.dart`, the `max_fee` field in `DeclareTransactionV2` is of type `String` instead of `Felt` because the devnet only supports a 16-byte length for this field, so we can't send this field as a `Felt` (it's too long).
🔇 Additional comments (78)
contracts/v2.6.2/Scarb.toml (2)
9-10: Consider adding an upper bound to the starknet dependency
Using >=2.6.0 without an upper bound could lead to compatibility issues if breaking changes are introduced in future versions. Consider using a version range like >=2.6.0, <2.7.0 to ensure compatibility.
Let's verify the compatibility of these versions:
15-17: Verify the necessity of all compiler output formats
Enabling all three output formats (sierra, casm, and caml) will increase build time and artifact size. Consider enabling only the formats required for your use case:
sierra: Required for contract declarationcasm: Required for contract deploymentcaml: Optional, used for debugging
Let's check if all formats are being used in the codebase:
packages/starknet/lib/src/contract/model/entry_points_by_type.freezed.dart (4)
30-30: LGTM: Added JSON serialization support.
The addition of toJson() methods across all data classes is consistent and follows Dart best practices for JSON serialization.
Also applies to: 260-260, 436-436, 670-670, 873-873, 1120-1120
33-34: LGTM: Added clear documentation for copyWith methods.
The added documentation comments improve code clarity by explaining the purpose of the copyWith methods consistently across all classes.
Also applies to: 62-63, 110-111, 204-205, 263-264, 289-290, 329-330, 384-385, 414-415, 439-440, 469-470, 519-520, 613-614, 652-653, 673-674, 699-700, 744-745, 815-816, 848-849, 876-877, 909-910, 961-962, 1058-1059, 1101-1102, 1123-1124, 1150-1151, 1193-1194, 1247-1248, 1277-1278
35-35: LGTM: Improved JSON serialization control.
The update from @JsonKey(ignore: true) to @JsonKey(includeFromJson: false, includeToJson: false) provides more explicit control over JSON serialization behavior.
Also applies to: 196-196, 246-246, 380-380, 417-417, 441-441, 605-605, 655-655, 675-675, 810-810, 851-851, 878-878, 1050-1050, 1104-1104, 1125-1125, 1243-1243, 1280-1280
Line range hint 1-1281: Verify freezed code generation.
As this is generated code, please ensure:
- The code was generated using the latest stable version of freezed
- The generation was performed after all model changes were completed
✅ Verification successful
Based on the output, I can generate the final response since we have verified the freezed version and checked for any uncommitted generated files.
Generated code is up-to-date and using latest freezed version
The code was generated with freezed v2.5.7 which is the latest stable version used in the starknet package. No uncommitted generated files were found, indicating the code generation is complete and properly tracked.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify freezed version and check for any uncommitted generated files
# Check freezed version in pubspec.yaml
rg "freezed:" -A 1
# Check for any uncommitted generated files
fd ".freezed.dart$" | xargs git status --porcelain
Length of output: 807
packages/starknet_provider/lib/src/model/get_transaction.freezed.dart (3)
Line range hint 874-1221: Generated Code Detected: Manual Modifications Not Required
As per previous learnings, this code segment is part of a file (get_transaction.freezed.dart) automatically generated by the freezed package. It should not be manually modified.
Line range hint 1222-1356: Generated Code Detected: Manual Modifications Not Required
This code segment is auto-generated. Manual changes are not recommended.
Line range hint 1357-1698: Generated Code Detected: Manual Modifications Not Required
As learned previously, modifications to generated code (.freezed.dart files) should be avoided.
packages/starknet/lib/src/crypto/model/pedersen_params.freezed.dart (4)
30-32: LGTM: Enhanced JSON serialization support
The changes appropriately add JSON serialization support and improve documentation:
- Added
toJson()method for JSON serialization - Updated
@JsonKeyannotations to be more explicit about serialization behavior - Added clear documentation for the
copyWithfunctionality
Also applies to: 227-227, 274-277
33-34: LGTM: Improved documentation clarity
Documentation comments have been consistently added across all implementations, clearly explaining the purpose of the copyWith functionality.
Also applies to: 65-66, 131-132, 232-233, 274-275
Line range hint 1-277: Verify code generation is up-to-date
Since this is generated code, let's ensure it's up-to-date with the source file.
✅ Verification successful
Based on the verification results, I can now provide the final response:
Generated code is properly configured and up-to-date
The code is a properly generated Freezed file with correct configuration:
- The package has the required dependencies:
freezed: ^2.5.7andbuild_runner: ^2.4.13 - The build.yaml contains proper Freezed configuration
- The source file contains correct Freezed annotations and part declarations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the generated code is up-to-date
# Look for build.yaml configuration and verify the generation command
rg -l "build.yaml" --type yaml
cat $(rg -l "build.yaml" --type yaml) | grep -A 5 "freezed"
# Check if there are any uncommitted changes in the generated files
git status packages/starknet/lib/src/crypto/model/pedersen_params.freezed.dart
Length of output: 232
Script:
#!/bin/bash
# Let's verify the source file and its generation markers
cat packages/starknet/lib/src/crypto/model/pedersen_params.dart
# Check for freezed dependency in pubspec.yaml
fd pubspec.yaml | xargs cat | grep -A 5 "freezed"
# Look for build_runner commands in the repository
rg "build_runner" --type yaml
Length of output: 65712
Script:
#!/bin/bash
# Let's verify the freezed dependency and build_runner configuration
fd pubspec.yaml | xargs grep -A 10 "dependencies:"
# Check for build.yaml configuration
fd build.yaml | xargs cat
# Check if this is a generated file by looking at its git history
git log --oneline packages/starknet/lib/src/crypto/model/pedersen_params.freezed.dart
Length of output: 11366
Line range hint 1-2:
This file is automatically generated by the Freezed package. Any changes should be made to the source file pedersen_params.dart instead.
Let's verify the source file exists and contains the corresponding annotations:
✅ Verification successful
Based on the examination of the source file pedersen_params.dart, I can now generate the final response:
The source file pedersen_params.dart exists and contains the proper @freezed annotation with @JsonSerializable configuration. The .freezed.dart file is correctly generated from this source file. Any changes to JSON serialization should be made in the source file pedersen_params.dart instead.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the source file and its annotations
# Look for the source file and its Freezed annotations
rg -l "pedersen_params.dart" --type dart
rg "@freezed" -A 5 "pedersen_params.dart"
Length of output: 364
Script:
#!/bin/bash
# Let's search for the source file with a more flexible approach
fd "pedersen_params\.dart$" --type f --exclude "*.freezed.dart" --exclude "*.g.dart"
# If we find the file, let's examine its content
rg "@freezed" -A 5 --type dart
Length of output: 65722
Script:
#!/bin/bash
# Let's examine the content of the source file to verify its annotations
cat packages/starknet/lib/src/crypto/model/pedersen_params.dart
Length of output: 65602
packages/starknet/lib/src/crypto/model/poseidon_params.freezed.dart (2)
31-32: LGTM: Consistent updates to JSON serialization configuration
The changes appear to be from updating the Freezed package configuration:
- Added toJson() method for JSON serialization
- Updated JsonKey annotations from
ignore: trueto more specificincludeFromJson: false, includeToJson: false - Added consistent documentation for copyWith methods
These changes improve the JSON serialization configuration while maintaining backward compatibility.
Also applies to: 34-36, 67-68, 253-253, 265-267, 310-313
Line range hint 1-4:
This is an auto-generated file by the Freezed package. Any manual modifications will be overwritten when the code is regenerated.
Let's verify if this is the latest generated code matching the source:
✅ Verification successful
Let me gather more information about the source file and its generated counterpart to ensure they are in sync.
Based on the source file content and the presence of build_runner and freezed dependencies in the pubspec.yaml, I can now provide a final response:
Generated code matches source file annotations
The freezed.dart file is correctly generated from the source file's annotations. The source file properly defines:
@freezedannotation on thePoseidonParamsclass@JsonSerializableannotation withfieldRenameparameter- Part declarations for both .freezed.dart and .g.dart files
The warning about not modifying generated code is valid and standard practice.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the generated code matches the source
# Find the source file
fd -t f "poseidon_params.dart$" --exclude "*.freezed.dart" --exclude "*.g.dart"
# Check if build_runner is in pubspec.yaml
rg "build_runner:" -A 5
Length of output: 3164
Script:
#!/bin/bash
# Check the source file content and its last modification time
cat packages/starknet/lib/src/crypto/model/poseidon_params.dart
stat -c %y packages/starknet/lib/src/crypto/model/poseidon_params.dart packages/starknet/lib/src/crypto/model/poseidon_params.freezed.dart
# Check if there are any build_runner related tasks in the package
cd packages/starknet && cat Makefile 2>/dev/null || true
Length of output: 26131
packages/starknet/lib/src/contract/model/compiled_contract.freezed.dart (4)
29-31: Addition of toJson Method Enhances Serialization
The inclusion of the toJson method in SierraCompiledContract allows instances to be serialized to JSON, which is essential for data exchange and storage.
533-535: Addition of bytecodeSegmentLengths Improves Data Integrity
The addition of the bytecodeSegmentLengths property in CASMCompiledContract ensures that all necessary data is included for accurate serialization and deserialization. This aligns with previous learnings that all contracts have bytecodeSegmentLengths.
754-757: bytecodeSegmentLengths Parameter Correctly Marked as Required
The bytecodeSegmentLengths parameter in the _CASMCompiledContract constructor is now required, ensuring that instances cannot be created without this essential information, thus improving data integrity.
794-796: Addition of toJson Method in DeprecatedCompiledContract
Including the toJson method enhances the serialization capabilities of DeprecatedCompiledContract, facilitating easier data handling and interoperability.
packages/starknet_provider/lib/src/model/estimate_fee.g.dart (4)
150-197: Approved: Correct JSON serialization for BroadcastedInvokeTxnV3Impl
The JSON serialization and deserialization methods for BroadcastedInvokeTxnV3Impl are correctly implemented, ensuring proper handling of all fields.
370-417:
Data type inconsistency for tip field in BroadcastedDeployAccountTxnV3Impl
Similarly, in the BroadcastedDeployAccountTxnV3Impl class, the tip field is being handled as a String. For consistency with other numeric fields, tip should be treated as a Felt object.
Apply this diff to correct the issue:
Felt.fromJson(json['contract_address_salt'] as String),
feeDataAvailabilityMode: json['fee_data_availability_mode'] as String,
nonceDataAvailabilityMode:
json['nonce_data_availability_mode'] as String,
paymasterData: (json['paymaster_data'] as List<dynamic>)
.map((e) => Felt.fromJson(e as String))
.toList(),
resourceBounds: (json['resource_bounds'] as Map<String, dynamic>).map(
(k, e) =>
MapEntry(k, ResourceBounds.fromJson(e as Map<String, dynamic>)),
),
- tip: json['tip'] as String,
+ tip: Felt.fromJson(json['tip'] as String),
$type: json['starkNetRuntimeTypeToRemove'] as String?, 'resource_bounds':
instance.resourceBounds.map((k, e) => MapEntry(k, e.toJson())),
- 'tip': instance.tip,
+ 'tip': instance.tip.toJson(),
'starkNetRuntimeTypeToRemove': instance.$type,Ensuring consistent data types across the codebase helps prevent runtime errors and improves code maintainability.
⛔ Skipped due to learnings
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet_provider/lib/src/model/declare_transaction.dart:90-90
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `packages/starknet_provider/lib/src/model/declare_transaction.dart`, the `tip` field in `DeclareTransactionV3` should remain a `String` because the target node doesn't support a `Felt` type for this field.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/invoke_transaction.dart:88-88
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `InvokeTransactionV3` class in `packages/starknet_provider/lib/src/model/invoke_transaction.dart`, the `tip` field is defined as a `String` because the development environment doesn't support a `Felt` type for this field.
228-257:
Inconsistent handling of max_fee field in BroadcastedDeclareTxnV2Impl
In the BroadcastedDeclareTxnV2Impl class, the max_fee field is being treated as a String in both serialization and deserialization methods. In other transaction implementations, max_fee is handled as a Felt object. This inconsistency may lead to deserialization errors and type mismatches when processing transactions.
Apply this diff to correct the issue:
type: json['type'] as String,
- max_fee: json['max_fee'] as String,
+ maxFee: Felt.fromJson(json['max_fee'] as String),
version: json['version'] as String, 'type': instance.type,
- 'max_fee': instance.max_fee,
+ 'max_fee': maxFeeToJson(instance.maxFee),
'version': instance.version,Ensure that the class property is correctly named maxFee and that it is of type Felt to maintain consistency.
⛔ Skipped due to learnings
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet_provider/lib/src/model/declare_transaction.dart:0-0
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `packages/starknet_provider/lib/src/model/declare_transaction.dart`, the `max_fee` field in `DeclareTransactionV2` is of type `String` instead of `Felt` because the devnet only supports a 16-byte length for this field, so we can't send this field as a `Felt` (it's too long).
259-307:
Data type inconsistency for tip field in BroadcastedDeclareTxnV3Impl
The tip field is currently handled as a String in the serialization and deserialization methods of BroadcastedDeclareTxnV3Impl. Considering that tip represents a numeric value similar to nonce and senderAddress, which are handled as Felt, it's advisable to maintain consistency by treating tip as a Felt object.
Apply this diff to correct the issue:
senderAddress: Felt.fromJson(json['sender_address'] as String),
- tip: json['tip'] as String,
+ tip: Felt.fromJson(json['tip'] as String),
$type: json['starkNetRuntimeTypeToRemove'] as String?, 'sender_address': instance.senderAddress.toJson(),
- 'tip': instance.tip,
+ 'tip': instance.tip.toJson(),
'starkNetRuntimeTypeToRemove': instance.$type,This change ensures consistent data types across numeric fields and prevents potential serialization issues.
⛔ Skipped due to learnings
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet_provider/lib/src/model/declare_transaction.dart:90-90
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `packages/starknet_provider/lib/src/model/declare_transaction.dart`, the `tip` field in `DeclareTransactionV3` should remain a `String` because the target node doesn't support a `Felt` type for this field.
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet_provider/lib/src/model/invoke_transaction.dart:88-88
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `InvokeTransactionV3` class in `packages/starknet_provider/lib/src/model/invoke_transaction.dart`, the `tip` field is defined as a `String` because the development environment doesn't support a `Felt` type for this field.
packages/starknet/lib/src/contract/contract.dart (2)
1-1: Approved: Updated import statement improves clarity
The change from a relative import to a package import enhances code clarity and maintainability.
76-85: Verify that optional parameters are correctly handled in account.execute
Ensure that all the new optional parameters passed to account.execute are supported and properly handled within the Account class. This will prevent potential runtime errors due to unhandled parameters.
packages/starknet/lib/src/types/felt.dart (2)
42-48: 🛠️ Refactor suggestion
Handle subtraction underflow with modular reduction
Instead of throwing an error for negative results, apply modular reduction to correctly handle underflow in field arithmetic.
Apply this diff:
Felt operator -(Felt other) {
- Felt result = Felt(_bigInt - other._bigInt);
- if (result._bigInt < BigInt.zero) {
- throw ArgumentError('Value must be greater than 0');
- }
+ Felt result = Felt((_bigInt - other._bigInt) % prime);
return result;
}⛔ Skipped due to learnings
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet/lib/src/types/felt.dart:42-48
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In `packages/starknet/lib/src/types/felt.dart`, in the `Felt` class, the subtraction operator `operator -(Felt other)` should throw an error if the result is negative, rather than performing modular subtraction.
33-39: 🛠️ Refactor suggestion
Perform modular reduction in addition operator
To maintain correct field arithmetic, the addition operator should apply modular reduction instead of throwing an error when exceeding the prime.
Apply this diff:
Felt operator +(Felt other) {
- Felt result = Felt(_bigInt + other._bigInt);
- if (result._bigInt >= prime) {
- throw ArgumentError('Value must be smaller than 2^251 + 17 * 2^192 + 1');
- }
+ Felt result = Felt((_bigInt + other._bigInt) % prime);
return result;
}⛔ Skipped due to learnings
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet/lib/src/types/felt.dart:33-39
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the `Felt` class, when performing addition, if the result exceeds `prime`, an error should be thrown instead of performing modular reduction.
Learnt from: rukafe0x
PR: focustree/starknet.dart#411
File: packages/starknet/lib/src/types/felt.dart:15-22
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the `Felt` class (`packages/starknet/lib/src/types/felt.dart`), when implementing operators like `<<` and `>>`, if the result exceeds `prime`, an error should be thrown instead of performing modular reduction.
packages/starknet_provider/lib/src/model/declare_transaction.dart (5)
10-16: Consistent use of constants for transaction versions enhances maintainability
Defining constants for transaction versions improves readability and maintainability of the codebase.
28-43: Efficient version handling in factory constructor
The updated fromJson factory method now effectively handles multiple transaction versions with clearer error messages for unsupported versions.
54-54: Use of constant in DeclareTransactionV1 improves consistency
Setting the version field using the declareTxnV1 constant aligns with the new version handling approach.
72-74: Appropriate handling of max_fee as String in DeclareTransactionV2
Using String for max_fee due to devnet limitations is appropriate and aligns with the retrieved learnings.
86-109: Introduction of DeclareTransactionV3 and ResourceBounds extends functionality
Adding DeclareTransactionV3 with the required fields supports version 3 declare transactions, and the ResourceBounds class provides necessary structure for resource bounds.
packages/starknet_provider/lib/src/model/deploy_account_transaction.g.dart (1)
Line range hint 1-86: Autogenerated file - manual review not necessary
This file is autogenerated by the build process and should not be manually modified or reviewed.
packages/starknet_provider/lib/src/model/invoke_transaction.g.dart (1)
Line range hint 1-124: Autogenerated file - manual review not necessary
This file is autogenerated by the build process and should not be manually modified or reviewed.
packages/starknet_provider/lib/src/model/declare_transaction.g.dart (1)
Line range hint 1-138: Skip review of autogenerated file
According to the learnings, files ending with .g.dart are autogenerated and should not be manually modified or reviewed for changes.
packages/starknet_provider/lib/src/model/estimate_fee.dart (5)
10-12: Constants for transaction versions added correctly
The constants estimateFeeV0, estimateFeeV1, and estimateFeeV3 are defined appropriately, matching the expected transaction version identifiers.
104-123: BroadcastedInvokeTxnV3 correctly implemented without maxFee
The BroadcastedInvokeTxnV3 class is correctly defined for version 3 invoke transactions. As per the updated specifications and retrieved learnings, the maxFee field is not required in version 3 transactions, and it has been appropriately omitted.
157-177: BroadcastedDeclareTxnV3 correctly omits maxFee as per version 3 specifications
The BroadcastedDeclareTxnV3 class is properly defined for version 3 declare transactions, aligning with the retrieved learnings that maxFee is not required in version 3 declare transactions.
205-224: BroadcastedDeployAccountTxnV3 defined correctly without maxFee
The BroadcastedDeployAccountTxnV3 class is correctly implemented for version 3 deploy account transactions, adhering to the retrieved learnings that maxFee is not required in version 3 transactions.
228-240: Updated BroadcastedTxn.fromJson to handle version 3 transactions correctly
The fromJson factory method has been appropriately updated to handle version 3 transactions for DECLARE, DEPLOY, and INVOKE types, ensuring that the correct subclasses are instantiated based on the transaction version.
packages/starknet/lib/src/signer.dart (3)
15-88: signInvokeTransactionsV3 method correctly implements version 3 transaction signing
The signInvokeTransactionsV3 method is properly implemented, handling the new parameters for version 3 transactions, including resource bounds, account deployment data, paymaster data, and data availability modes. Input validation is appropriately performed, and the transaction hash computation follows the expected protocol.
287-356: signDeclareTransactionV3 method correctly handles version 3 declare transactions
The signDeclareTransactionV3 method accurately implements the signing process for version 3 declare transactions, including handling of resource bounds and data availability modes. The method includes necessary validation and correctly computes the transaction hash.
403-473: signDeployAccountTransactionV3 method properly implements version 3 deploy account transaction signing
The signDeployAccountTransactionV3 method is correctly implemented, handling version 3 deploy account transactions with the appropriate parameters and transaction hash computation.
packages/starknet/lib/src/contract/model/compiled_contract.dart (1)
226-245: Updated classHash method to handle new compiler versions and bytecode segment lengths
The classHash method in CASMCompiledContract now correctly accounts for compiler versions greater than or equal to 1.1.0 by adding the COMPILED_CLASS_V1 element when appropriate. It also handles the new bytecodeSegmentLengths parameter, adjusting the hash computation as per the updated specifications introduced in Sierra 1.5.0.
packages/starknet/test/account_test.dart (5)
72-127: Tests for declaring Cairo 1 contracts are well-implemented
The tests for declaring simple Sierra contracts with provided CASM files are correctly implemented and cover important scenarios.
128-179: STRK fee handling in declaration tests is appropriate
The inclusion of STRK fee support and resource bounds in the declaration tests is correctly handled.
330-331: Verify the correctness of amount calculation for STRK token transfer
In the amount calculation for transferring STRK tokens, you multiply maxFee.maxAmount and maxFee.maxPricePerUnit. Please verify that this multiplication yields the correct amount and that the units are consistent. Multiplying two Felt values may not produce the intended result if units or scales differ.
Line range hint 365-484: Exception handling in invoke test is appropriate
The try-catch blocks correctly handle exceptions related to contract errors, allowing the tests to proceed when appropriate.
486-618: STRK fee usage in invoke v3 test is correctly implemented
The test for invoking a function with STRK fee and invoke v3 is well-structured and utilizes the STRK fee parameters appropriately.
packages/starknet_provider/test/integration/read_provider_test.dart (2)
Line range hint 991-1050: New test for estimating fee with V1 Invoke transaction is valid
The test for estimating the fee of a V1 Invoke transaction is correctly implemented and includes appropriate assertions for fee components.
1090-1107: Handle different error codes in fee estimation
The test correctly anticipates either CONTRACT_NOT_FOUND or CONTRACT_ERROR responses from the devnet. Good job accommodating different possible error codes.
packages/starknet/lib/src/account.dart (1)
631-728: Check for nullability and type safety
Ensure that all parameters in the declare method are properly checked for null values and that type casting is safe. This will prevent runtime errors during contract declarations.
packages/starknet_provider/lib/src/model/deploy_account_transaction.freezed.dart (2)
31-32: Update version string for compatibility
The comment indicates that 0x1 is used instead of 0x01 for devnet compatibility. Ensure that this change does not affect mainnet transactions where 0x01 might be expected.
354-789:
Ensure new transaction version V3 is fully integrated
The addition of DeployAccountTransactionV3 introduces new fields and logic. Verify that all serializers, deserializers, and related logic handle this new version correctly throughout the codebase.
✅ Verification successful
Let me analyze the output and generate additional verification scripts to ensure complete coverage.
Based on the comprehensive output, I can now generate the final response as we have sufficient evidence of proper integration.
DeployAccountTransactionV3 integration is properly implemented
The transaction version V3 is fully integrated with appropriate handling throughout the codebase:
- Proper version constants defined (
deployAccountTxnV3,deployAccountTxnV3OldCompat) - Complete serialization/deserialization implementation in generated code
- Version-aware transaction handling in estimate fee logic
- Dedicated signer implementation with
signDeployAccountTransactionV3 - Proper resource bounds and data availability modes support
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for usages of DeployAccountTransactionV3 and ensure proper handling.
# Find all files that reference DeployAccountTransactionV3
grep -r "DeployAccountTransactionV3" packages/
# Check serialization methods
grep -r "fromJson" packages/ | grep "DeployAccountTransactionV3"
grep -r "toJson" packages/ | grep "DeployAccountTransactionV3"
Length of output: 10144
Script:
#!/bin/bash
# Check for transaction version constants and their usage
rg -A 2 "deployAccountTxnV3" packages/
# Check for any transaction version handling logic
rg -A 5 "version.*=.*V3" packages/
# Check for any transaction type handling or validation
ast-grep --pattern 'match $expr {
$$$
"DEPLOY_ACCOUNT" => $$$
$$$
}'
# Check for signer implementation
rg -A 10 "signDeployAccountTransactionV3" packages/starknet/lib/src/signer.dart
Length of output: 8798
packages/starknet_provider/lib/src/model/invoke_transaction.freezed.dart (3)
350-350: Use of constants for version fields improves maintainability
The change to use the constant invokeTxnV0 for the version field enhances code consistency and maintainability by avoiding hardcoded strings.
656-656: Use of constants for version fields improves maintainability
Similarly, using the constant invokeTxnV1 for the version field improves code consistency and reduces potential errors from hardcoded values.
781-1215: Ensure comprehensive support for InvokeTransactionV3
The addition of InvokeTransactionV3 introduces support for version 3 invoke transactions with new fields. Please verify that all parts of the codebase that handle InvokeTransaction instances (e.g., serialization, deserialization, transaction processing, validation logic) have been updated to accommodate this new version.
Run the following script to check for handling of InvokeTransactionV3 throughout the codebase:
✅ Verification successful
Let me gather more information about the transaction handling across the codebase.
Let me search for more specific transaction handling implementations.
Based on the gathered information, I can now provide the final response:
InvokeTransactionV3 support is properly implemented
The codebase shows comprehensive support for InvokeTransactionV3:
- Proper serialization/deserialization with version handling (
invokeTxnV3andinvokeTxnV3OldCompat) - Complete implementation in the transaction factory with version-based routing
- Integration with fee estimation (
BroadcastedInvokeTxnV3) - Usage in account transactions (
account.dart) - Full support for all V3-specific fields (accountDeploymentData, feeDataAvailabilityMode, etc.)
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for usage of InvokeTransaction and ensure InvokeTransactionV3 is handled.
# Search for factory methods or functions handling InvokeTransaction versions
rg -A 5 -B 5 'InvokeTransaction' --type dart | rg -B 2 -A 2 'invokeTxnV3'
Length of output: 2057
Script:
#!/bin/bash
# Search for transaction processing and validation logic
rg -A 5 "class.*Transaction.*Provider|interface.*Transaction.*Provider" --type dart
# Search for any transaction version handling or validation
rg -A 3 "version.*=.*invokeTxn|case.*invokeTxn" --type dart
# Search for transaction serialization/deserialization
rg -A 3 "fromJson.*InvokeTransaction|toJson.*InvokeTransaction" --type dart
Length of output: 3752
Script:
#!/bin/bash
# Search for transaction handling in provider implementations
rg -A 5 "class.*Provider.*implements" --type dart
# Search for any InvokeTransactionV3 specific handling
rg -A 3 "InvokeTransactionV3" --type dart
# Search for transaction version constants and their usage
rg -l "invokeTxnV3" --type dart | xargs rg -A 5 "invokeTxnV3"
Length of output: 23900
packages/starknet_provider/lib/src/model/declare_transaction.freezed.dart (3)
498-499: Clarify the change of max_fee type to String in DeclareTransactionV2
The max_fee field in DeclareTransactionV2 has been changed from Felt to String. Based on previous learnings, this adjustment addresses devnet limitations on field length. Ensure that all usages and serializations of max_fee in DeclareTransactionV2 correctly handle the String type.
714-715: Consistent handling of max_fee as String
The max_fee field is now a String in the implementation of DeclareTransactionV2. This aligns with the need to support devnet's 16-byte length limitation. Confirm that this change is consistently applied and properly documented in the codebase.
828-1296: Confirm fee handling in DeclareTransactionV3
The DeclareTransactionV3 class introduces new fields related to fee handling, such as feeDataAvailabilityMode and resourceBounds, but does not include a max_fee field like previous versions. Please verify that fee management for version 3 transactions is correctly implemented according to the latest specifications.
Run the following script to search for references to fee handling in DeclareTransactionV3:
✅ Verification successful
Based on the search results, I can now provide a final response:
Fee handling in DeclareTransactionV3 is correctly implemented
The implementation correctly follows the latest specifications by replacing the max_fee field with the new fee model that includes:
resourceBounds: A map containing L1 and L2 resource bounds, where each bound specifiesmaxAmountandmaxPricePerUnitfeeDataAvailabilityMode: Defaults to 'L1' for fee data availabilitytip: A new field for optional tips to sequencers
The implementation is consistent with other V3 transaction types (Invoke, Deploy) and is properly integrated with the fee estimation system.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that `DeclareTransactionV3` fee handling is appropriately implemented.
# Search for `DeclareTransactionV3` implementations and check for fee-related fields.
rg -A 10 'class.*DeclareTransactionV3' --type dart
# Search for usages of `max_fee` in relation to `DeclareTransactionV3`
rg 'DeclareTransactionV3.*max_fee' --type dart
Length of output: 8860
Script:
#!/bin/bash
# Let's perform a more comprehensive check of fee handling in DeclareTransactionV3
# Check the base DeclareTransaction interface/class for fee handling
rg -A 5 "class DeclareTransaction" --type dart
# Search for any fee-related constants or version-specific fee handling
rg -l "declareTxnV3|max_fee|resourceBounds|feeDataAvailabilityMode" --type dart
# Look for documentation or comments about fee handling in V3
rg -B 2 -A 2 "V3.*fee|fee.*V3" --type dart
# Check for any tests related to DeclareTransactionV3 fee handling
rg "test.*DeclareTransactionV3.*fee|test.*fee.*DeclareTransactionV3" --type dart
Length of output: 12988
Script:
#!/bin/bash
# Let's check the implementation details of resource bounds and fee handling
# Check the ResourceBounds class definition
rg -A 10 "class ResourceBounds" --type dart
# Look for documentation or implementation of fee data availability modes
rg -A 5 "feeDataAvailabilityMode.*=|DATA_AVAILABILITY_MODE" --type dart
# Check for any constants or enums related to V3 fee handling
rg -A 5 "const.*declareTxnV3|enum.*DataAvailabilityMode" --type dart
# Look for the base DeclareTransaction interface to understand fee handling across versions
ast-grep --pattern 'abstract class DeclareTransaction {
$$$
}'
Length of output: 26849
packages/starknet/lib/src/contract/model/contract_abi.freezed.dart (3)
148-154: Addition of toJson() methods enhances serialization capabilities
The inclusion of toJson() methods in the mixins for various ABI entries (e.g., _$SierraContractAbiEntry) improves the ability to serialize these objects into JSON, which is essential for contract interaction and data exchange.
335-337: Ensure proper exclusion of copyWith in JSON serialization
The annotation @JsonKey(includeFromJson: false, includeToJson: false) on copyWith methods prevents them from being included during JSON serialization, which is appropriate to avoid unnecessary data in the JSON representation.
497-500: Verify consistent application of copyWith annotations
The copyWith methods have been annotated to exclude them from JSON serialization. Please ensure that this annotation is consistently applied across all relevant classes to maintain serialization integrity.
Run the following script to check for consistent annotations:
✅ Verification successful
The previous command might have failed because we were piping too aggressively. Let's try a broader search to understand the context better and then narrow down our verification strategy based on the results.
The previous output showed many copyWith methods, but we need to specifically verify if they consistently have the @JsonKey(includeFromJson: false, includeToJson: false) annotation. This command will help us see the relationship between the annotations and copyWith methods more clearly.
Based on the search results, I can now provide a final response. The output shows that all copyWith methods in the freezed.dart files consistently have the @JsonKey(includeFromJson: false, includeToJson: false) annotation, including the one in the file under review. This is a consistent pattern across the codebase.
Annotation consistency for copyWith methods verified
The @JsonKey(includeFromJson: false, includeToJson: false) annotation is consistently applied to all copyWith methods across the codebase's freezed.dart files, confirming that the serialization behavior is uniform.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that `copyWith` methods are properly annotated across classes.
# Search for `copyWith` methods without the `JsonKey` annotation.
rg -A 2 'copyWith' --type dart | rg -B 2 -A 2 'copyWith' | rg -L '@JsonKey'
Length of output: 71
Script:
#!/bin/bash
# Let's try a different approach to find copyWith methods and their annotations
# First, let's find all files that contain copyWith methods
rg -l "copyWith" --type dart
# Then, let's examine the context around copyWith methods more broadly
rg -B 5 -A 2 "copyWith" --type dart
Length of output: 68987
Script:
#!/bin/bash
# Let's search for copyWith methods specifically in freezed.dart files and check their JsonKey annotations
rg -B 3 "copyWith" --type dart --glob "*freezed.dart" | rg -B 3 "@JsonKey|copyWith"
Length of output: 81364
packages/starknet_provider/lib/src/model/fee_estimate.dart (1)
8-14: Update FeeEstimate constructor to include additional fee metrics
The FeeEstimate class constructor now includes additional required parameters: dataGasConsumed, dataGasPrice, and unit. This change allows for a more detailed fee estimation, capturing nuanced aspects of transaction costs.
packages/starknet_provider/lib/src/model/fee_estimate.g.dart (1)
12-16: LGTM! New fields for v3 transaction support are properly serialized.
The generated code correctly handles the new fields required for v3 transactions:
- dataGasConsumed
- dataGasPrice
- unit
The JSON field names follow the correct snake_case convention.
Also applies to: 22-26
✅ Verification successful
Generated code matches source class definition perfectly
The generated JSON deserialization code in fee_estimate.g.dart exactly matches the fields defined in the source class FeeEstimate in fee_estimate.dart:
dataGasConsumeddataGasPricegasPriceoverallFeeunit
All fields are properly typed as String and follow the correct snake_case JSON naming convention.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the source class definition matches the generated code
# Look for the source file that defines FeeEstimate class
fd -e dart -E "fee_estimate\.dart$" --exclude "*.g.dart"
Length of output: 12285
Script:
#!/bin/bash
# Let's find and check the source file for FeeEstimate class
ast-grep --pattern 'class FeeEstimate {
$$$
}'
# Also check the content of the source file
rg -l "FeeEstimate" packages/starknet_provider/lib/src/model/fee_estimate.dart
cat packages/starknet_provider/lib/src/model/fee_estimate.dart
Length of output: 776
packages/starknet/lib/src/presets/udc.g.dart (1)
36-47: LGTM! Parameters are properly forwarded to execute method.
The implementation correctly forwards all v3-specific parameters to the execute method using named arguments.
packages/starknet/test/types/felt_test.dart (5)
12-19: LGTM! Addition operator tests are comprehensive.
Tests properly verify:
- Basic addition functionality
- Overflow condition with max value
20-26: LGTM! Subtraction operator tests align with previous requirements.
Tests correctly verify:
- Basic subtraction
- Error condition for negative results, as specified in previous learnings
27-33: LGTM! Right shift operator tests cover essential cases.
Tests verify:
- Basic right shift operation
- Multiple boundary conditions
34-40: LGTM! Left shift operator tests handle overflow correctly.
Tests properly verify:
- Basic left shift operation
- Overflow condition with max value
41-50: LGTM! Prime value edge case is properly tested.
Test verifies that creating a Felt with a value equal to prime throws an ArgumentError with the correct message.
contracts/v2.6.2/src/counter.cairo (1)
1-40: LGTM! Well-structured trait and implementation.
The Counter contract provides a clean implementation with clear method signatures and straightforward state management.
packages/starknet/lib/src/contract/erc20.dart (1)
57-58: LGTM! Improved method call structure and error handling
The changes consistently use named parameters for execute calls and fix the error message in the approve method, improving code clarity and debugging experience.
Also applies to: 76-77, 94-95, 102-102
packages/starknet/lib/src/static_config.dart (2)
29-32: LGTM! Well-documented STRK token address addition
The STRK token address is properly documented with a clear comment about network compatibility.
34-75: Verify devnet version compatibility
The account configurations are updated for devnet 0.1.2. Ensure that the development environment is using the compatible version.
✅ Verification successful
Devnet version compatibility confirmed
The account configurations are correctly aligned with starknet-devnet 0.1.2, which is specified in both .env.devnet and .tool-versions files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check devnet version in environment files
echo "Checking devnet version in environment files..."
rg -l "STARKNET_DEVNET_VERSION.*0\.1\.2" .env*
# Check tool versions
echo "Checking tool versions..."
cat .tool-versions | grep "starknet-devnet"
Length of output: 303
packages/starknet/lib/src/contract/model/compiled_contract.g.dart (1)
64-67: LGTM: Proper implementation of bytecodeSegmentLengths serialization.
The new field is correctly implemented with proper type conversion from JSON numbers to Dart integers.
Also applies to: 76-76
packages/starknet/test/model.freezed.dart (1)
26-31: LGTM: Documentation and annotation improvements.
The changes enhance code clarity through better documentation and more explicit JSON serialization annotations.
Also applies to: 140-146, 170-174
contracts/v1/artifacts/contract2_Counter.compiled_contract_class.json (1)
1-1: LGTM! Auto-generated contract artifact.
This is a correctly generated contract artifact file containing the Counter contract implementation. The file includes all necessary components: prime field specification, bytecode, hints, and proper entry points for external calls and constructor.
Let's verify the compiler version matches the project requirements:
packages/starknet_provider/lib/src/model/deploy_account_transaction.dart
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
packages/starknet_provider/lib/src/model/deploy_account_transaction.dart (3)
10-13: LGTM! Consider adding documentation.The version constants are well-defined and include both new and backward-compatible versions.
Consider adding documentation comments to explain the difference between regular and OldCompat versions:
+/// Version identifier for Deploy Account Transaction V1 const String deployAccountTxnV1 = '0x1'; +/// Version identifier for Deploy Account Transaction V3 const String deployAccountTxnV3 = '0x3'; +/// Backward compatibility version for Deploy Account Transaction V1 const String deployAccountTxnV1OldCompat = '0x01'; +/// Backward compatibility version for Deploy Account Transaction V3 const String deployAccountTxnV3OldCompat = '0x03';
17-27: Enhance error message for unsupported versions.The version handling looks good, but the error message could be more descriptive.
Consider improving the error message to be more informative:
- _ => throw Exception("Unsupported version ${json['version']}"), + _ => throw Exception( + "Unsupported deploy account transaction version ${json['version']}. " + + "Supported versions are: $deployAccountTxnV1, $deployAccountTxnV3, " + + "$deployAccountTxnV1OldCompat, $deployAccountTxnV3OldCompat" + ),
52-73: Add field validation and documentation for V3 transaction.The V3 transaction implementation looks good but could benefit from additional validation and documentation.
Consider these improvements:
- Add validation for data availability modes:
@Assert("feeDataAvailabilityMode == 'L1' || feeDataAvailabilityMode == 'L2'") @Assert("nonceDataAvailabilityMode == 'L1' || nonceDataAvailabilityMode == 'L2'")
- Consider using an enum for data availability modes:
enum DataAvailabilityMode { L1, L2 }
Consider using a numeric type for tip instead of String.
Add documentation for new fields:
/// Transaction class for StarkNet Deploy Account Transaction V3 /// /// Contains additional fields for enhanced fee handling and data availability: /// * [feeDataAvailabilityMode] - Layer specification for fee data ('L1' or 'L2') /// * [nonceDataAvailabilityMode] - Layer specification for nonce data ('L1' or 'L2') /// * [paymasterData] - Data required by the paymaster /// * [resourceBounds] - Bounds for various resources used in the transaction /// * [tip] - Additional fee provided to the sequencerpackages/starknet/test/account_test.dart (3)
76-79: Consider using path constants for contract artifacts.The contract artifact paths are duplicated across multiple tests. Consider extracting them into constants at the top of the file.
However, based on the retrieved learning from PR #412, code duplication of contract artifact paths in test files is acceptable, so this suggestion can be ignored.
409-414: Improve error handling specificity.The error handling could be more specific. Instead of checking if the error message contains 'Contract error', consider using a more specific error type or error code.
-if (!e.toString().contains('Contract error')) { +if (e is! ContractError && e is! ClassHashAlreadyDeclared) { rethrow; }
620-622: Consider adjusting the timeout duration.A 10-minute timeout for integration tests might be too long. Consider reducing it or making it configurable through environment variables.
-timeout: Timeout(Duration(minutes: 10)) +timeout: Timeout(Duration(minutes: const int.fromEnvironment('TEST_TIMEOUT_MINUTES', defaultValue: 5)))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
contracts/v1/artifacts/contract2_Counter2.compiled_contract_class.json(1 hunks)contracts/v1/artifacts/contract2_Counter2.contract_class.json(1 hunks)packages/starknet/test/account_test.dart(2 hunks)packages/starknet_provider/lib/src/model/deploy_account_transaction.dart(2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/starknet/test/account_test.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/test/account_test.dart:63-65
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the test files such as 'packages/starknet/test/account_test.dart', code duplication of contract artifact paths is acceptable, and refactoring suggestions to extract constants or helper methods are not required.
🔇 Additional comments (5)
packages/starknet_provider/lib/src/model/deploy_account_transaction.dart (1)
43-44: LGTM! Version default is correctly updated.
The default version now uses the constant and maintains devnet compatibility as documented.
contracts/v1/artifacts/contract2_Counter2.contract_class.json (1)
1-1: LGTM! The contract class is well-structured.
The contract implements a standard counter interface with proper method signatures, event handling, and initialization. The ABI is well-defined and follows best practices.
contracts/v1/artifacts/contract2_Counter2.compiled_contract_class.json (1)
1-1: LGTM! The compiled contract class is properly structured.
The compiled contract includes well-defined entry points, properly segmented bytecode, and appropriate hints for the Cairo VM.
packages/starknet/test/account_test.dart (2)
83-92: LGTM! Proper fee estimation before declaration.
The code correctly estimates the maximum fee before declaring the contract, which is a good practice to prevent transaction failures.
584-591: LGTM! Comprehensive transaction configuration.
The execute call includes important parameters for transaction handling:
- Non-legacy calldata format
- Nonce increment on errors
- Multiple retry attempts
- STRK fee configuration
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (3)
packages/starknet_provider/lib/src/model/deploy_account_transaction.dart (3)
10-13: Add documentation for version constantsConsider adding documentation comments to explain:
- The difference between regular and OldCompat versions
- When each version should be used
- Which StarkNet protocol versions they correspond to
+/// Version identifier for Deploy Account Transaction V1 +/// Used in StarkNet protocol version X.X and above const String deployAccountTxnV1 = '0x1'; +/// Version identifier for Deploy Account Transaction V3 +/// Used in StarkNet protocol version X.X and above const String deployAccountTxnV3 = '0x3'; +/// Backwards compatibility version identifier for Deploy Account Transaction V1 +/// Used in older StarkNet protocol versions const String deployAccountTxnV1OldCompat = '0x01'; +/// Backwards compatibility version identifier for Deploy Account Transaction V3 +/// Used in older StarkNet protocol versions const String deployAccountTxnV3OldCompat = '0x03';
17-25: Improve error handling in factory methodThe current error handling uses a generic Exception. Consider:
- Using a more specific exception type
- Including valid version values in the error message
switch (json['version']) { deployAccountTxnV1 => DeployAccountTransactionV1.fromJson(json), deployAccountTxnV1OldCompat => DeployAccountTransactionV1.fromJson(json), deployAccountTxnV3 => DeployAccountTransactionV3.fromJson(json), deployAccountTxnV3OldCompat => DeployAccountTransactionV3.fromJson(json), - _ => throw Exception("Unsupported version ${json['version']}"), + _ => throw ArgumentError( + 'Unsupported version ${json['version']}. ' + 'Supported versions are: $deployAccountTxnV1, $deployAccountTxnV3, ' + '$deployAccountTxnV1OldCompat, $deployAccountTxnV3OldCompat' + ), };
66-66: Consider using numeric type for tipThe
tipfield is defined as String but represents a numeric value. Consider using a more appropriate type likeFeltfor consistency with other numeric fields in the codebase.- required String tip, + required Felt tip,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/starknet_provider/lib/src/model/deploy_account_transaction.dart(2 hunks)
🔇 Additional comments (1)
packages/starknet_provider/lib/src/model/deploy_account_transaction.dart (1)
41-42: LGTM!
The version update is correct and the comment provides helpful context about devnet compatibility.
packages/starknet_provider/lib/src/model/deploy_account_transaction.dart
Show resolved
Hide resolved
packages/starknet_provider/lib/src/model/deploy_account_transaction.dart
Show resolved
Hide resolved
…declare a simple sierra contract with provided CASM file
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
packages/starknet/test/account_test.dart (4)
76-79: Consider extracting contract paths to constants.The contract paths are duplicated across multiple tests. While code duplication in tests is acceptable as per the learnings, extracting these paths to constants would make maintenance easier if the paths need to change.
Also applies to: 131-134
409-414: Improve error handling specificity.The error handling catches all errors and only checks if they contain 'Contract error'. Consider using more specific error types or error codes for better test reliability.
-if (!e.toString().contains('Contract error')) { +// Check for specific error codes or types +if (e is! ContractError && !e.toString().contains('Contract error')) {
297-300: Consider using meaningful constants for test values.The test uses magic numbers for private keys and other values. Consider extracting these to named constants with meaningful descriptions.
+const testAccountPrivateKey = "0x12345678abcdef"; +const testAccountInitialBalance = 100; -final accountPrivateKey = Felt.fromHexString("0x12345678abcdef"); +final accountPrivateKey = Felt.fromHexString(testAccountPrivateKey);
585-590: Document the rationale for maxAttempts value.The test uses
maxAttempts: 5without explaining why this value was chosen. Consider adding a comment explaining the reasoning or extracting it to a named constant.+// Maximum number of retry attempts for nonce-related errors +const maxNonceRetryAttempts = 5; -maxAttempts: 5, +maxAttempts: maxNonceRetryAttempts,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/starknet/lib/src/static_config.dart(1 hunks)packages/starknet/test/account_test.dart(2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/starknet/test/account_test.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/test/account_test.dart:63-65
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the test files such as 'packages/starknet/test/account_test.dart', code duplication of contract artifact paths is acceptable, and refactoring suggestions to extract constants or helper methods are not required.
🔇 Additional comments (2)
packages/starknet/lib/src/static_config.dart (1)
29-32: LGTM! Addition of STRK token address.
The addition of the STRK token address constant is well-documented and necessary for v3 operations with STRK fee payments.
packages/starknet/test/account_test.dart (1)
Line range hint 72-622: Comprehensive test coverage for v3 functionality.
The test suite thoroughly covers all aspects of the new v3 functionality including:
- Declaration with both ETH and STRK fees
- Deployment with resource bounds
- Invocation with new parameters
- Fee estimation for all operations
The tests are well-structured and provide good coverage of the new features.
…ace condition with other groups
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/starknet/test/account_test.dart (3)
76-79: Consider extracting contract paths to constantsThe contract artifact paths are duplicated across multiple test cases. Consider extracting them to constants at the file level for better maintainability.
Note: Based on the learnings from PR #412, code duplication of contract artifact paths in test files is acceptable, so this is just a nitpick suggestion.
296-361: Consider extracting common deployment test logicThe account deployment test for v3 largely duplicates the v1 test logic. Consider extracting common deployment steps into helper methods to reduce duplication and improve maintainability.
Example helper method:
Future<void> verifyAccountDeployment({ required Felt classHash, required Felt accountAddress, required Account deployerAccount, required bool useSTRKFee = false, }) async { final accountClassHash = await getAccountClassHash(accountAddress, deployerAccount.provider); expect(accountClassHash, equals(classHash)); }
450-452: Consider documenting magic numbersThe test uses magic numbers (100, 0) for token amounts. Consider adding comments explaining what these values represent or extract them into named constants for better readability.
+ // Transfer 100 tokens with 0 high bits (amount stored as uint256) account1.accountAddress, Felt.fromInt(100), // amount low Felt.fromInt(0), // amount high
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/starknet/test/account_test.dart(2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/starknet/test/account_test.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/test/account_test.dart:63-65
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the test files such as 'packages/starknet/test/account_test.dart', code duplication of contract artifact paths is acceptable, and refactoring suggestions to extract constants or helper methods are not required.
🔇 Additional comments (2)
packages/starknet/test/account_test.dart (2)
585-590: LGTM: Comprehensive v3 transaction testing
The test properly verifies v3 transaction features with:
- Legacy calldata handling
- Nonce error recovery
- Multiple attempts
- STRK fee support
- Resource bounds
Line range hint 72-622: Overall test structure and coverage is excellent
The new test groups provide comprehensive coverage of v3 functionality:
- Declaration with resource bounds
- Account deployment with STRK fees
- Contract execution with v3 features
- Proper error handling and retries
The tests are well-structured and properly validate the new features described in the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
packages/starknet/test/account_test.dart (1)
Line range hint
379-414: Improve error handling in contract declarationThe error handling could be more specific. Instead of catching all errors and checking the error message string, consider:
- Using specific error types
- Adding more descriptive error messages
- Logging the full error context
try { // ... declaration code ... } catch (e) { - print(e.toString()); - if (!e.toString().contains('Contract error')) { + if (e is ContractError) { + // Contract already declared, continue + print('Contract already declared: ${e.message}'); + } else { + print('Unexpected error during contract declaration: $e'); rethrow; } }Also applies to: 499-537
🧹 Nitpick comments (4)
packages/starknet/test/account_test.dart (4)
77-79: Consider extracting contract paths to constantsThe hardcoded contract artifact paths are duplicated across test cases. Consider extracting these paths to constants at the top of the file for better maintainability.
+ const String COUNTER_CONTRACT_PATH = '/../../contracts/v1/artifacts/contract2_Counter2'; + const String TOKEN_CONTRACT_PATH = '/../../contracts/v1/artifacts/contract2_MyToken'; + + String getContractPath(String basePath, String suffix) => + '${Directory.current.path}$basePath.$suffix'; - '${Directory.current.path}/../../contracts/v1/artifacts/contract2_Counter2.contract_class.json' + getContractPath(COUNTER_CONTRACT_PATH, 'contract_class.json')Also applies to: 132-134
83-92: Consider extracting common declaration logicThe contract declaration logic is duplicated between test cases with only minor differences in fee handling. Consider extracting this into a helper method to reduce duplication.
Future<String> declareContract({ required Account account, required CompiledContract sierraContract, required BigInt compiledClassHash, bool useSTRKFee = false, }) async { var maxFee = await account.getEstimateMaxFeeForDeclareTx( compiledContract: sierraContract, compiledClassHash: compiledClassHash, useSTRKFee: useSTRKFee, ); var res = await account.declare( compiledContract: sierraContract, compiledClassHash: compiledClassHash, useSTRKFee: useSTRKFee, l1MaxAmount: useSTRKFee ? maxFee.maxAmount : null, l1MaxPricePerUnit: useSTRKFee ? maxFee.maxPricePerUnit : null, max_fee: useSTRKFee ? null : maxFee.maxFee, ); return res.when( result: (result) => result.transactionHash.toHexString(), error: (error) => throw error, ); }Also applies to: 139-151
227-230: Consider using meaningful test valuesThe test uses magic numbers for private and public keys. Consider using named constants with meaningful values or generating these values dynamically.
+ const TEST_ACCOUNT_PRIVATE_KEY = '0x12345678'; + const TEST_ACCOUNT_PUBLIC_KEY = '0x47de619de131463cbf799d321b50c617566dc897d4be614fb3927eacd55d7ad'; - final accountPrivateKey = Felt.fromHexString('0x12345678'); + final accountPrivateKey = Felt.fromHexString(TEST_ACCOUNT_PRIVATE_KEY);Also applies to: 297-300
585-591: Document the purpose of execution parametersThe execute function includes several parameters whose purpose isn't immediately clear. Consider adding a comment explaining their significance, especially for parameters like
useLegacyCalldataandincrementNonceIfNonceRelatedError.+ // Parameters for handling transaction retries and fee estimation: + // - useLegacyCalldata: Use old calldata format for backwards compatibility + // - incrementNonceIfNonceRelatedError: Automatically retry with incremented nonce + // - maxAttempts: Maximum number of retry attempts + // - useSTRKFee: Use STRK token for fee payment final response = await account0.execute( functionCalls: [functionCall], useLegacyCalldata: false, incrementNonceIfNonceRelatedError: true, maxAttempts: 5, useSTRKFee: true, l1MaxAmount: maxFee.maxAmount, l1MaxPricePerUnit: maxFee.maxPricePerUnit, );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/starknet/test/account_test.dart(2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/starknet/test/account_test.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/test/account_test.dart:63-65
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the test files such as 'packages/starknet/test/account_test.dart', code duplication of contract artifact paths is acceptable, and refactoring suggestions to extract constants or helper methods are not required.
🔇 Additional comments (1)
packages/starknet/test/account_test.dart (1)
620-622: Consider adjusting timeout duration
The 2-minute timeout might be too long for CI/CD pipelines. Consider making this configurable or reducing it unless there's a specific reason for such a long timeout.
✅ Verification successful
Let me search for more context about test timeouts in the codebase.
Let me search specifically for integration test files and their timeout patterns.
Based on the gathered information, I can now provide the final response:
2-minute timeout is appropriate for these integration tests
The timeout of 2 minutes is justified for these integration tests because:
- These are Starknet blockchain integration tests that involve network operations
- Other integration tests in the codebase are also marked with similar timeouts
- The tests involve complex operations like contract deployment and execution
- The test is specifically tagged as an integration test, not a unit test
- The project's CI configuration (melos.yaml) shows these integration tests are run separately from unit tests
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if other integration tests have similar timeout values
rg -A 1 "timeout: Timeout" .
Length of output: 305
Script:
#!/bin/bash
# Search for all test files and their timeout configurations
rg -B 2 "test\(.*\)" . | rg -B 2 "timeout|minutes"
# Also check for any test configuration files
fd -e yaml -e yml | xargs rg "timeout|test"
Length of output: 2072
Script:
#!/bin/bash
# Search for integration test files
fd -e dart test$ | xargs rg "tags.*integration" -B 5 -A 5
Length of output: 10467
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
packages/starknet/test/account_test.dart (1)
Line range hint
365-484: Consider breaking down large test casesThese test cases are quite long and handle multiple concerns (declaration, deployment, and execution). Consider breaking them down into smaller, focused test cases using helper methods.
Example structure:
Future<Felt> declareContract(CompiledContract contract) async { // Declaration logic } Future<Felt> deployContract(Felt classHash, List<Felt> constructorCalldata) async { // Deployment logic } test('succeeds to declare and verify cairo 1 contract', () async { final classHash = await declareContract(sierraContract); // Verify declaration }); test('succeeds to deploy declared contract', () async { final contractAddress = await deployContract(classHash, constructorCalldata); // Verify deployment }); test('succeeds to execute function on deployed contract', () async { // Execute and verify });Also applies to: 486-618
🧹 Nitpick comments (5)
packages/starknet/lib/src/static_config.dart (2)
34-36: Consider enhancing version documentation.The class hash update for devnet 0.1.2 is correct. However, to improve maintainability, consider adding a link to the devnet version's release notes or documentation.
68-95: Consider security implications of exposed private keys.While these are devnet accounts, it's recommended to:
- Add a prominent warning comment that these are for testing only
- Consider moving private keys to environment variables or a secure configuration file
packages/starknet/test/account_test.dart (3)
83-86: Consider extracting fee estimation into a helper methodThe fee estimation logic is duplicated across multiple tests. Consider extracting it into a helper method to improve maintainability and reduce code duplication.
+ Future<FeeEstimations> estimateDeclareFee({ + required CompiledContract compiledContract, + required BigInt compiledClassHash, + bool useSTRKFee = false, + }) async { + return account2.getEstimateMaxFeeForDeclareTx( + compiledContract: compiledContract, + compiledClassHash: compiledClassHash, + useSTRKFee: useSTRKFee, + ); + }Also applies to: 139-143
110-125: Consider using a custom matcher for contract verificationThe contract verification logic is complex and could be made more readable using a custom matcher.
Matcher matchesContract(CompiledContract contract) { return predicate((SierraContractClass actual) { return equals(contract.contract.sierraProgram.map((e) => Felt(e))) .matches(actual.sierraProgram, {}); }); } // Usage in test: expect(contract, matchesContract(sierraContract));
297-298: Consider using named constants for test keysHard-coded private/public keys should be extracted into named constants for better maintainability and clarity.
+ const TEST_ACCOUNT_PRIVATE_KEY = "0x12345678abcdef"; + const TEST_ACCOUNT_PUBLIC_KEY = "0x44702ae20646bbb316ee2f301c9b31ca9f7f301d48d2b6ee82da71f828e8bcb"; - final accountPrivateKey = Felt.fromHexString("0x12345678abcdef"); - final accountPublicKey = Felt.fromHexString("0x44702ae20646bbb316ee2f301c9b31ca9f7f301d48d2b6ee82da71f828e8bcb"); + final accountPrivateKey = Felt.fromHexString(TEST_ACCOUNT_PRIVATE_KEY); + final accountPublicKey = Felt.fromHexString(TEST_ACCOUNT_PUBLIC_KEY);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/starknet/lib/src/static_config.dart(1 hunks)packages/starknet/test/account_test.dart(2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/starknet/test/account_test.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/test/account_test.dart:63-65
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the test files such as 'packages/starknet/test/account_test.dart', code duplication of contract artifact paths is acceptable, and refactoring suggestions to extract constants or helper methods are not required.
🔇 Additional comments (3)
packages/starknet/lib/src/static_config.dart (2)
29-32: LGTM! Verify STRK token address.
The STRK token address constant is correctly defined and follows the established pattern.
✅ Verification successful
STRK token address verified and correctly implemented
The STRK token address (0x04718f5a0fc34cc1af16a1cdee98ffb20c31f5cd61d6ab07201858f4287c938d) is verified to be correct as it is:
- Documented in the project's examples (docs/examples/starknet-counter.mdx)
- Consistently used across the codebase in wallet balance checks and token transfers
- Properly referenced in both the static configuration and wallet provider implementation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify STRK token address against StarkNet documentation or deployments
# Search for any references to the STRK token address in markdown files or configs
rg -i "0x04718f5a0fc34cc1af16a1cdee98ffb20c31f5cd61d6ab07201858f4287c938d" --type md
Length of output: 84
Script:
#!/bin/bash
# Let's try a broader search for STRK token references and its address
rg -i "strk.*address|0x04718f5a0fc34cc1af16a1cdee98ffb20c31f5cd61d6ab07201858f4287c938d" -A 2 -B 2
# Also search for any test files that might be using this address
fd -e test.dart -e spec.dart -X grep -l -i "strk"
Length of output: 2453
43-104: LGTM! Verify account configurations.
The account additions and updates are well-structured and the comment explains their purpose for parallel testing.
✅ Verification successful
The search results show that account2 and account3 are actively used in test files, particularly in packages/starknet/test/account_test.dart. They are used for various operations including declaring, deploying, and executing transactions, which aligns with the comment's explanation about needing additional accounts for parallel testing.
Let me verify if these accounts are used in isolated test groups to prevent nonce race conditions as intended.
Let me try a different approach to check the test structure:
Let me check specifically how account2 and account3 are used in their test contexts:
Account configurations are properly implemented and utilized
The verification confirms that account2 and account3 are correctly used in separate test scenarios, preventing nonce race conditions:
- account2 is dedicated to declaration tests for Cairo contracts
- account3 is used in a separate group of tests for deployment and execution operations
The account configurations align with the stated purpose in the comment about parallel test execution.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify account configurations in test files
# Check if the new accounts are properly utilized in tests
rg "account[2-3]" --type dart
Length of output: 2971
Script:
#!/bin/bash
# Check the test group structure and account usage context
rg -B 2 -A 2 "describe|test group" packages/starknet/test/account_test.dart
Length of output: 77
Script:
#!/bin/bash
# Check the test file content to understand the structure
rg "group|test\(" -A 3 packages/starknet/test/account_test.dart
Length of output: 4383
Script:
#!/bin/bash
# Get more context around account2 and account3 usage
rg -B 5 -A 5 "account[23]\." packages/starknet/test/account_test.dart
Length of output: 7661
packages/starknet/test/account_test.dart (1)
585-587: 🛠️ Refactor suggestion
Validate retry parameters
The retry parameters should be validated to ensure they don't exceed reasonable limits. Consider adding assertions or using constants for these values.
+ const MAX_RETRY_ATTEMPTS = 5;
+ assert(maxAttempts <= MAX_RETRY_ATTEMPTS, 'Max attempts should not exceed $MAX_RETRY_ATTEMPTS');
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
packages/starknet/test/account_test.dart (3)
79-82: Consider using path constants for contract artifactsThe contract artifact paths are hardcoded and duplicated across multiple test cases. This could make maintenance difficult if the paths change.
Consider extracting these paths into constants at the top of the file:
const String CONTRACT_ARTIFACTS_PATH = '${Directory.current.path}/../../contracts/v1/artifacts'; const String COUNTER_CONTRACT_PATH = '$CONTRACT_ARTIFACTS_PATH/contract2_Counter2'; const String MYTOKEN_CONTRACT_PATH = '$CONTRACT_ARTIFACTS_PATH/contract2_MyToken';
415-421: Consider using more specific error handlingThe current error handling relies on string matching which is fragile. While the previous review comment about this was addressed, we could still improve the error handling to be more specific.
Consider using a more specific error check:
- if (!e.toString().contains('Contract error')) { + if (e is! ContractAlreadyDeclaredError) { rethrow; }
474-484: Consider adding balance verification helperThe balance verification logic is duplicated across multiple test cases.
Consider extracting this into a helper method:
Future<int> getTokenBalance(Account account, Felt contractAddress, Felt accountAddress) async { final result = await account.provider.call( request: FunctionCall( contractAddress: contractAddress, entryPointSelector: getSelectorByName("balance_of"), calldata: [accountAddress] ), blockId: BlockId.latest, ); return result.when( result: (result) => result[0].toInt(), error: (error) => throw Exception("Failed to get balance"), ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/starknet/test/account_test.dart(2 hunks)
🧰 Additional context used
📓 Learnings (1)
packages/starknet/test/account_test.dart (1)
Learnt from: rukafe0x
PR: focustree/starknet.dart#412
File: packages/starknet/test/account_test.dart:63-65
Timestamp: 2024-11-12T04:15:58.361Z
Learning: In the test files such as 'packages/starknet/test/account_test.dart', code duplication of contract artifact paths is acceptable, and refactoring suggestions to extract constants or helper methods are not required.
🔇 Additional comments (4)
packages/starknet/test/account_test.dart (4)
85-94: LGTM: Fee estimation implementation looks good
The implementation correctly estimates the maximum fee before declaring the contract and uses the estimated fee in the declaration transaction.
592-595: LGTM: Robust transaction retry mechanism
Good implementation of transaction retry logic with:
useLegacyCalldata: falsefor v3 supportincrementNonceIfNonceRelatedError: truefor handling nonce issuesmaxAttempts: 5for retry attempts
546-565: LGTM: STRK fee support implementation
The implementation correctly handles STRK fee estimation and payment for deployment transactions with:
- Proper fee estimation using
useSTRKFee: true - Correct usage of
l1MaxAmountandl1MaxPricePerUnit
568-579: LGTM: Fee estimation for invoke transactions
The implementation correctly estimates fees for invoke transactions with STRK support.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
melos.yaml (1)
125-125: LGTM! Good call on limiting test concurrency.Adding
-c 1to run integration tests sequentially is a crucial change, especially given the stateful nature of StarkNet operations (declare, deploy, invoke) and fee estimation. This prevents potential race conditions and ensures reliable test execution.This change follows best practices for integration testing by:
- Ensuring test isolation
- Preventing resource contention in the devnet
- Maintaining deterministic test behavior
All tested locally in devnet 0.1.2 with "melos test" and "act -j lint".
(close issues #339 #340 #341 #342)
Summary by CodeRabbit
New Features
BroadcastedInvokeTxnV3, with additional fields for fee estimation.Accountclass to support new transaction version (v3) with enhanced fee estimation capabilities.executemethod in theContractclass for improved transaction configurations.FeeEstimateclass to include additional metrics for gas consumption and pricing.DeclareTransactionV3andDeployAccountTransactionV3.Bug Fixes
Documentation
Tests