refactor(e2e): unify submit helpers behind a typed SubmitError - #2013
refactor(e2e): unify submit helpers behind a typed SubmitError#2013wbaxterh wants to merge 1 commit into
Conversation
tests/e2e/src/api/midnight.rs had submit_midnight_tx (raw progress handle) and submit_expecting_rejection (drove it to a rejection, returning a stringly error). Add a typed SubmitError (Subxt for submission-time failures, NotFinalized for a watched tx that failed pre_dispatch/execution) and collapse them into one submit_midnight_tx that submits, waits for finalized success, and returns the finalized ExtrinsicEvents or a typed SubmitError. Callers expecting success use ?/expect; callers expecting a rejection match on the Err. submit_expecting_rejection had no callers and is removed. submit_expecting_success keeps its best-block-inclusion semantics via a shared submit_and_watch helper and now also returns SubmitError. The c2m_bridge caller uses the returned events directly. Assisted-by: Claude:claude-opus-4-8 Signed-off-by: wbaxterh <wesleybaxterhuber@gmail.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 353c186fc4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .await? | ||
| .wait_for_finalized_success() | ||
| .await | ||
| .map_err(|e| SubmitError::NotFinalized(e.to_string())) |
There was a problem hiding this comment.
When the transaction subscription disconnects, event retrieval fails, or another subxt/RPC error occurs while wait_for_finalized_success() is running, this mapping reports SubmitError::NotFinalized just like a pre-dispatch or execution rejection. A rejection-focused e2e test matching that variant can therefore pass during an infrastructure failure instead of proving the node rejected the transaction; preserve subxt watcher errors separately and reserve NotFinalized for confirmed transaction failures.
Useful? React with 👍 / 👎.
1 participant
Overview
Closes #1840.
Follow-up from the #1796 review.
tests/e2e/src/api/midnight.rshadsubmit_midnight_tx(returned the rawTransactionProgress) andsubmit_expecting_rejection(drove it to a rejection and returned a stringlyBox<dyn Error>). This adds a typedSubmitErrorand collapses them into a singlesubmit_midnight_txthat submits, waits for finalized success, and returns the finalizedExtrinsicEventsor a typed error, so callers match on the result instead of picking between two methods:SubmitError::Subxt— a subxt-layer failure, including the node rejecting the tx at submission time (e.g. "already imported" / "temporarily banned").SubmitError::NotFinalized— the tx was watched but failed pre_dispatch/execution.Notes on scope:
submit_expecting_rejectionhad no callers, so it is removed; a rejection is now the typedErrfromsubmit_midnight_tx.submit_expecting_successkeeps its best-block-inclusion semantics (no added latency on the deploy path) via a sharedsubmit_and_watchhelper, and now also returnsSubmitError.c2m_bridgecaller uses the returned finalized events directly.thiserror(already a workspace dependency) to the e2e crate.🗹 TODO before merging
📌 Submission Checklist
git commit -s) for the DCO🧪 Testing Evidence
Verified with
cargo test -p midnight-node-e2e --test e2e_tests -- --list(builds the e2e lib + test binary and lists tests): compiles clean, test set unchanged. No behavior change to the tests themselves; this is an internal helper refactor.🔱 Fork Strategy
Links
Closes #1840