Skip to content

feat(predict): add confirmation hook plumbing#29914

Merged
matallui merged 2 commits into
mainfrom
predict/dw-confirmation-hooks
May 8, 2026
Merged

feat(predict): add confirmation hook plumbing#29914
matallui merged 2 commits into
mainfrom
predict/dw-confirmation-hooks

Conversation

@matallui
Copy link
Copy Markdown
Contributor

@matallui matallui commented May 8, 2026

Description

This PR adds minimal Predict confirmation hook plumbing needed by the upcoming Polymarket Deposit Wallet migration.

It wires TransactionController confirmation lifecycle hooks to PredictController while keeping Predict behavior as passthrough by default:

  • beforePublish delegates to PredictController.beforePublish, which currently returns true.
  • publish delegates to PredictController.publish before Transaction Pay / 7702 / Smart Transactions, and continues normal publishing when Predict returns no transaction hash.
  • If a future Predict publish implementation returns { transactionHash, isIntentComplete: true }, TransactionController marks the latest transaction meta as isIntentComplete before returning the hash.

This PR intentionally contains no Polymarket Deposit Wallet business logic. It is a small foundation PR for confirmation-team review.

Changelog

CHANGELOG entry: null

Related issues

Fixes: N/A — preparatory plumbing for the Predict Deposit Wallet migration.

Manual testing steps

Feature: Predict confirmation hook plumbing

  Scenario: non-Predict transactions continue through the normal publish flow
    Given a transaction is published through TransactionController
    And PredictController.publish returns no transaction hash

    When the publish hook runs
    Then Transaction Pay / 7702 / Smart Transaction publishing continues as before

  Scenario: Predict publish can complete a transaction intent
    Given PredictController.publish returns a transaction hash and isIntentComplete

    When the publish hook runs
    Then normal publishing is short-circuited
    And the latest TransactionController transaction meta is marked intent complete

Local validation run:

yarn jest app/core/Engine/controllers/transaction-controller/transaction-controller-init.test.ts app/components/UI/Predict/controllers/PredictController.test.ts --runInBand
yarn lint:tsc

Screenshots/Recordings

N/A — no UI changes.

Before

N/A

After

N/A

Pre-merge author checklist

Performance checks (if applicable)

  • I've tested on Android
    • N/A — hook plumbing only, no UI/runtime performance path manually exercised.
  • I've tested with a power user scenario
    • N/A — hook plumbing only, no account/token rendering path changed.
  • I've instrumented key operations with Sentry traces for production performance metrics
    • N/A — this PR only adds passthrough hook plumbing.

For performance guidelines and tooling, see the Performance Guide.

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Note

Medium Risk
Touches transaction publishing lifecycle by adding new hooks and a short-circuit path, which could affect submission ordering and integration with Pay/7702/Smart Transactions if miswired. Default behavior remains passthrough, reducing blast radius.

Overview
Adds Predict-specific confirmation hook plumbing into the transaction submission lifecycle. TransactionController init now calls PredictController:beforePublish as a new hooks.beforePublish, and calls PredictController:publish at the start of hooks.publish, short-circuiting the rest of the publish pipeline when Predict returns a transactionHash.

Updates PredictController to expose new messenger methods (beforePublish, publish) with default passthrough implementations, extends messenger action typings/permissions accordingly, and adds unit tests verifying delegation, call ordering, and the short-circuit behavior.

Reviewed by Cursor Bugbot for commit 3f9d618. Bugbot is set up for automated code reviews on this repo. Configure here.

@metamaskbotv2 metamaskbotv2 Bot added the team-predict Predict team label May 8, 2026
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes.

@matallui matallui marked this pull request as ready for review May 8, 2026 12:29
@matallui matallui requested review from a team as code owners May 8, 2026 12:29
@github-actions github-actions Bot added the size-M label May 8, 2026
Comment thread app/core/Engine/controllers/transaction-controller/transaction-controller-init.ts Outdated
Comment thread app/core/Engine/controllers/transaction-controller/transaction-controller-init.ts Outdated
Comment thread app/core/Engine/controllers/transaction-controller/transaction-controller-init.ts Outdated
Comment thread app/core/Engine/controllers/transaction-controller/transaction-controller-init.ts Outdated
@matallui matallui requested a review from a team as a code owner May 8, 2026 15:35
@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented May 8, 2026

🔍 Smart E2E Test Selection

  • Selected E2E tags: SmokePredictions, SmokeConfirmations, SmokeWalletPlatform
  • Selected Performance tags: None (no tests recommended)
  • Risk Level: medium
  • AI Confidence: 88%
click to see 🤖 AI reasoning details

E2E Test Selection:
The changes introduce two new hooks (beforePublish and publish) into the PredictController and wire them into the TransactionController's transaction lifecycle via the messenger system.

Key impacts:

  1. SmokePredictions (primary): The PredictController now has beforePublish and publish hooks that integrate with the TransactionController's publish flow. When a Predict transaction is published, the PredictController:publish hook is called first - if it returns a transaction hash, it short-circuits the normal publish flow. This directly affects Polymarket prediction market transaction flows (opening positions, cashing out, claiming winnings).

  2. SmokeConfirmations (required by SmokePredictions tag description): The beforePublish and publish hooks are integrated into the core TransactionController publish pipeline. Every transaction now calls these hooks. The beforePublish hook is called before signing, and publish is called in the publishHook before pay/smart transaction hooks. While the default implementations are pass-throughs, this is a change to the critical transaction path that warrants confirmation flow testing.

  3. SmokeWalletPlatform (required by SmokePredictions tag description): Predictions is a section inside the Trending tab, so changes to Predictions views/flows affect Trending. The tag description explicitly states to select SmokeWalletPlatform when selecting SmokePredictions.

Risk factors:

  • The publishHook now calls PredictController:publish first and short-circuits if a hash is returned - this is a new code path in the critical transaction publish flow
  • The messenger wiring adds new action registrations to the TransactionControllerInitMessenger
  • Default implementations are pass-throughs (low risk for non-Predict transactions)
  • Changes are well-tested with unit tests covering the new behavior

Performance Test Selection:
The new beforePublish and publish hooks are simple pass-through implementations by default (returning true and { transactionHash: undefined } respectively). While they add async calls to the transaction publish flow, the overhead is negligible for non-Predict transactions. No performance tests are warranted for these changes.

View GitHub Actions results

@sonarqubecloud
Copy link
Copy Markdown

sonarqubecloud Bot commented May 8, 2026

Copy link
Copy Markdown
Contributor

@caieu caieu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matallui matallui enabled auto-merge May 8, 2026 16:56
Copy link
Copy Markdown
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@matallui matallui added this pull request to the merge queue May 8, 2026
Merged via the queue into main with commit e5a8b17 May 8, 2026
100 checks passed
@matallui matallui deleted the predict/dw-confirmation-hooks branch May 8, 2026 18:35
@github-actions github-actions Bot locked and limited conversation to collaborators May 8, 2026
@metamaskbotv2 metamaskbotv2 Bot added the release-7.78.0 Issue or pull request that will be included in release 7.78.0 label May 8, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

release-7.78.0 Issue or pull request that will be included in release 7.78.0 size-M team-predict Predict team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants