Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Cancellation currently stops awaiting but doesn’t abort the underlying RPC/HTTP requests, which can leave in-flight work running after navigation and undermines the intended reliability guarantees.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR hardens transaction-confirmation polling so the Sending page can recover from transient network/payload failures and be safely left/reopened without an older poll corrupting a newer session’s UI state.
Changes:
- Extracted a bounded
TransactionConfirmationpoller that retries read-only RPC status queries across transient failures and treats unknown execution status as pending. - Updated
SendingPageto use the extracted poller and to scope cancellation to each page appearance to prevent stale polls from updating state. - Added a dedicated
tests/p2-04test project covering recovery, pending/unknown receipts, HALT/FAULT outcomes, and cancellation/restart behavior.
File summaries
| File | Description |
|---|---|
tests/p2-04/TransactionConfirmationTests.cs |
Adds regression coverage for transient failures, pending vs HALT/FAULT, null results, and cancellation/restart. |
tests/p2-04/TransactionConfirmation.Tests.csproj |
Introduces a focused xUnit test project for the poller logic. |
OneGateApp/Services/RPC/TransactionConfirmation.cs |
New reusable confirmation poller with bounded retries and tolerant parsing/exception handling. |
OneGateApp/Pages/SendingPage.xaml.cs |
Replaces inline polling with the new poller and introduces per-appearance cancellation handling. |
OneGateApp.slnx |
Registers the new tests/p2-04 test project in the solution. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+18
to
+22
| JsonObject? tx = await read("getrawtransaction").WaitAsync(cancellationToken); | ||
| if (tx is null) continue; | ||
| blockTime = tx["blocktime"]?.GetValue<ulong>(); | ||
| if (!blockTime.HasValue) continue; | ||
| JsonObject? log = await read("getapplicationlog").WaitAsync(cancellationToken); |
Comment on lines
+16
to
+18
| <ItemGroup> | ||
| <ProjectReference Include="../../OneGateApp/OneGateApp.csproj" ReferenceOutputAssembly="false" BuildReference="false" SkipGetTargetFrameworkProperties="true" /> | ||
| </ItemGroup> |
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
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.
Summary
Fix audit P2-04: leaving the Sending page now cancels the underlying transaction-confirmation HTTP request instead of only cancelling an outer wait.
RpcSendAsyncoverload and pass the token toHttpClient.SendAsyncand JSON body deserialization.TransactionConfirmationreads for both transaction and application-log polling.Validation
dotnet test tests/p2-04/TransactionConfirmation.Tests.csproj --no-restore --nologo: 8/8 passed.OneGateApprebuilt fornet10.0-ios, installed and launched on the iPhone 17 simulator; normal welcome UI displayed.onegate_api36emulator: self-contained APK rebuilt withEmbedAssembliesIntoApk=true, installed and launched directly through the resolved MAUIMainActivity; normal welcome UI displayed./tmp.The cancellation regression is covered by the focused test project; simulator validation exercised the resulting production app startup on both platforms. No wallet signing, game play, or chain submission was performed.
This PR remains independently based on
master@623603d; no historical Erik-rejected approach or STOP item was changed.