fix: waitForAcceptance now chek on finality status #479
Conversation
|
To view this pull requests documentation preview, visit the following URL: docs.page/focustree/starknet.dart~479 Documentation is deployed and generated using docs.page. |
WalkthroughThe update refactors transaction status handling by introducing a new class Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant WT as waitForTransaction
participant WES as _waitForExecutionStatus
participant WTS as _waitForTransactionStatus
participant S as _Status
C->>WT: Call waitForTransaction(...)
WT->>WES: Invoke _waitForExecutionStatus(...)
WES->>WTS: Delegate execution status check
WTS->>S: Update status from receipt
S-->>WTS: Return updated execution status
WTS-->>WES: Pass execution result
WES-->>WT: Return result
WT-->>C: Return execution status
sequenceDiagram
participant C as Client
participant WA as waitForAcceptance
participant WFS as _waitForFinalityStatus
participant WTS as _waitForTransactionStatus
participant S as _Status
C->>WA: Call waitForAcceptance(...)
WA->>WFS: Invoke _waitForFinalityStatus(...)
WFS->>WTS: Delegate finality status check
WTS->>S: Update status from receipt
S-->>WTS: Return updated finality status
WTS-->>WFS: Pass finality result
WFS-->>WA: Return result
WA-->>C: Return finality status
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🔇 Additional comments (9)
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: 0
🧹 Nitpick comments (5)
packages/starknet/lib/src/util.dart (5)
20-24: Consider using an enum or sealed class for statuses.
Using plain strings increases the risk of typos and ambiguity. An enum (or similar) would help ensure type safety and clarity across the codebase.
26-31: Add documentation comments for_waitForTransactionStatus.
This method is pivotal in orchestrating transaction status checks. Including doc comments clarifying the purpose and parameters (especiallycheckStatus) will improve readability and maintainability.
45-58: Reduce repetition when assigning_statusfrom receipt properties.
All non-pending receipts share the same assignment pattern. Consider refactoring into a helper function or single unified mapping to eliminate repeated lines and ease maintenance.
87-88: Check completeness of_errorStatesand consider exponential backoff.
_errorStatescurrently only includes'REVERTED'. If other error statuses (e.g.,'REJECTED') exist, consider adding them.- Using a constant interval is fine for now, but exponential backoff might improve performance and reduce network load.
Also applies to: 92-92, 96-96
161-161: Fix minor spelling in documentation.
“Succeeded” is spelled with a double “c”.-/// Returns `true` when [transactionHash] execution status is in `{PENDING, SUCEEDED}` +/// Returns `true` when [transactionHash] execution status is in `{PENDING, SUCCEEDED}`Also applies to: 165-165
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/starknet/lib/src/util.dart(6 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: analyze
- GitHub Check: lint
- GitHub Check: test-integration
🔇 Additional comments (8)
packages/starknet/lib/src/util.dart (8)
37-37: Initialization defaults look fine.
Using_Status('UNKNOWN', 'UNKNOWN')as a fallback when transaction data is unavailable is reasonable.
77-77: Consistent fallback forTXN_HASH_NOT_FOUND.
Re-initializing_statustoUNKNOWNaligns with the existing approach in line 37. No concerns here.
101-117:_waitForExecutionStatusis implemented cleanly.
The delegation to_waitForTransactionStatuswithexecutionfield checks is straightforward.
119-135:_waitForFinalityStatuscorrectly parallels_waitForExecutionStatus.
The approach provides a consistent pattern for finality checks.
137-159:waitForStatewrapper logic is coherent.
The usage of_waitForExecutionStatusenforcing the specified states is clear and maintains simplicity.
175-175: Usage of['PENDING', 'SUCCEEDED']is correct.
Matches the documentation once spelling is addressed. No other issues found.Also applies to: 178-178
187-187: Documentation updates for acceptance logic are well-defined.
They outline the finality-based acceptance and retry approach clearly.Also applies to: 191-191
202-202:waitForFinalityStatususage for acceptance checks is consistent.
Ensures the transaction is truly “accepted” by verifying it reachesACCEPTED_ON_L1orACCEPTED_ON_L2.Also applies to: 204-204
Close #441
Summary by CodeRabbit