Fix encoding version in typed data Domain
#228
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
franciszekjob
changed the title
StarknetTypedData version type to Stringversio in typed data Domain
franciszekjob
changed the title
versio in typed data Domainversion in typed data Domain
franciszekjob
commented
Apr 18, 2025
Comment on lines
-50
to
+57
| "version": 1, | ||
| "version": "1", | ||
| "chainId": 2137, | ||
| } | ||
| """ | ||
| static let exampleDomainV1 = """ | ||
| { | ||
| "name": "DomainV1", | ||
| "version": 2, | ||
| "version": "2", |
There was a problem hiding this comment.
note: We wouldn't have to change this, but our tests compare original json and the one after decoding+encoding, hence they fail (version in original is int and while in encoded it's string).
DelevoXDG
reviewed
Apr 18, 2025
Comment on lines
522
to
533
| func toString() throws -> String { | ||
| switch self { | ||
| case let .string(s): return s | ||
| case let .decimal(n): return String(n) | ||
| case let .signedDecimal(n): return String(n) | ||
| case let .felt(f): return f.toHex() | ||
| case let .signedFelt(f): return f.toHex() | ||
| case let .bool(b): return String(b) | ||
| default: | ||
| throw StarknetTypedDataError.encodingError | ||
| } | ||
| } |
There was a problem hiding this comment.
That seems a bit Java-esque to me.
Shouldn't we instead be using CustomStringConvertible conformance and overriding description?
That allows us to create string in a more swift-native way, via String(describing:)
There was a problem hiding this comment.
Valid suggestion
franciszekjob
merged commit 1291c82
into
feat/219-2-remove-old-txs-write-api
1 check passed
franciszekjob
added a commit
that referenced
this pull request
Apr 18, 2025
This reverts commit 1291c82.
franciszekjob
added a commit
that referenced
this pull request
Apr 18, 2025
franciszekjob
added a commit
that referenced
this pull request
Apr 18, 2025
* Support storage proof * Move `ContractStorageKey` * Support `starknet_getMessagesStatus` * Adapt execution resources * Support failure reason in transaction status * Remove unnecessary coding key change in `StarknetFeeEstimate` * Add `StarknetResourceBoundsMapping.zero` * Fix `testEstimateInvokeV3Fee` * Restore `==` in `BinaryNode` * Update Sources/Starknet/Crypto/FeeCalculation.swift Co-authored-by: Maksim Zdobnikau <[email protected]> * Make `contractAddresses` and `contractStorageKeys` optional in `getStorageProof` * Add merkle node tests * Refactor `init` in `MerkleNode` * Add `==` in `MerkleNode` * Format * Update values in `MerkleNodeTests` * Add todo * Fix typo in `contracts_storage_keys` * Rename `ContractStorageKey` to `ContractStorageKey` * Make `blockId` optional * Make `blockId` optional in `getStorageProof` * Change `path` type to `NumAsHex` * Revert optional block id changes * Update `StarknetFeeEstimate` description * Update CI * Update devnet client * Update `StarknetFeeEstimate` extension * Add l1 data gas to `StarknetResourceBoundsMapping` * Adjust tests * Include l1 data gas in tx hash calculations * Add `testGetStorageProof` * Fix models and field names * Bump starknet foundry to 0.40.0 in `.tool-versions` * wip: adjust devnet client * Bump action versions * Fix linting * Fix linting * Temporarily comment out running tests * Revert "Temporarily comment out running tests" This reverts commit 6b7f7f4. * Formatting * Update permissions * Fix for account type * Fix formatting * Fix `prefundAccount` in `DevnetClientProtocol` * Add `UNIVERSAL_SIERRA_COMPILER` env * Fix `testEstimateMessageFee` * Fix `testGetEvents` * Update echo in workflow * Remove echo * Fix `testBatchGetTransactionByHash` * Comment out failing tests * Fix `UNIVERSAL_SIERRA_COMPILER` env * Fix `testStarknetCallsToExecuteCalldataCairo1` * Fix `testEdgeNode` * Fix `TransactionTests` * Remove v1 methods and corresponding tests * Formatting * Formatting * Fix formatting * Remove unneeded structs and methods * Add `storageRoot` to `ContractLeafData` * Add `isReverted` to `StarknetFunctionInvocation` * Uncomment tests * Remove prints * Fix `testSimulateTransactionsV3` * Restore prefund amount as uint128 * Update readme * Devnet client cleanup * Restore setting initial balance * Fix `deployAccount` params * Fix formatting * Rename file with `ContractsStorageKeys` * Apply code review suggestions * Update todo * Uncomment tests for getting txs * Bump macos to 15 * Use xcode 16.2.0 * Fix `testSimulateTransactionsV1` * Update fee estimate field types * Fix `testDeployAccountV1` * Remove unused variable * Fic typo * Restore comment * Add `Starknet` prefix to struct names * Refactor `FeeEstimateTests` * Remove `UNIVERSAL_SIERRA_COMPILER` from workflow * Fix multiplier * Remove default * Remove `UNIVERSAL_SIERRA_COMPILER` from readme * Apply code review suggestions * Remove usc entry from readme * Add `testGetStorageProofRequest` * Add `Starknet` prefix to `NodeHashToNodeMapping` * Apply code review suggestions * Apply code review suggestion * Remove `Scarb.lock` * Fix encoding `version` in typed data `Domain` (#228) * Change `StarknetTypedData` version type to `String` * Remove dummy changes * Add `Domain` encoding * Apply code review suggestion * Little refactor * Little refactor * Remove unused code * Fix version encoding * Fix tests * Revert "Fix encoding `version` in typed data `Domain` (#228)" (#229) This reverts commit 1291c82. --------- Co-authored-by: Maksim Zdobnikau <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Describe your changes
versioninDomainis now encoded as string, instead of numberLinked issues
Closes #227
Breaking changes