Feature/safe queue

[Oxbobby]

Summary

Introduces the controller and library support required by the Safe Queue, including recoverable rejection, same-nonce transaction resolution, Safe Global refresh, and queue-aware portfolio simulation.

What changed

Safe request lifecycle

• Removed the option to Reject a partially or fully signed Safe transaction
• Added the option to Cancel a partially or fully signed Safe transaction. Cancel means initiating an onchain transaction replacement with an alternative transaction with the same nonce that sends 0 value to address 0

Same-nonce resolution

• Broadcasting a Safe transaction removes all competing transactions for the same account, network, and nonce.
• Automatically resolved transactions are persisted with account and network scope.
• Removed requests do not automatically open the next signing request.
• Failed or stuck broadcasts only restore matching transactions from the same account, network, and nonce.

Portfolio simulation

• Dashboard simulation has been stopped for partially or fully signed Safe transaction. Only pending to be signed transaction are simulated on the dashboard (ones you are building right now) with the exception of the one transaction that you have opened in SignAccountOp (if any)

Safe Global integration

• Added a guarded manual refresh operation with controller status reporting.
• Duplicate refresh actions are prevented while a refresh is already running.

Other behavior

• Dashboard banners are only generated for Safe transactions that have not collected signatures.
• Added a shared Safe-aware account-op nonce helper.

Automated coverage

Added coverage for:

• sequential Safe simulation across gaps and duplicate nonces
• same-nonce transaction discovery and cleanup
• account/network isolation of automatically resolved transactions
• manual refresh concurrency

[Oxbobby]

/review

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🔒 No security concerns identified

📝 TODO sections src/controllers/requests/requests.ts [939]: double check if we have a condition when to fire simulation and when not to src/controllers/main/main.ts [665]: the form should be reset in a success state in FE

⚡ Recommended focus areas for review Resource leak In removeUserRequests, when a Safe transaction is auto-resolved (shouldRejectSafeRequests is false), the request is spliced from this.userRequests and added to safeResolveIds, but req.signAccountOp.destroy() is never called. The old code always called destroy() for Safe requests. This leaks the SignAccountOpController (subscriptions, timers, listeners) in the long-lived background process, accumulating over time. const safeNonce = getAccountOpNonce(req.signAccountOp.accountOp) // if it's not a safe request OR it's non-signed Safe request, move on if ( !req.signAccountOp.account.safeCreation || !req.signAccountOp.accountOp.txnId || safeNonce === null ) { req.signAccountOp.destroy() return } // handle removing a safe transaction // if it's not signed, we destroy it // if we're rejecting it, we write it as rejected in storage but pause // the signing process // if we're auto-resolving it (same nonce txn already broadcast), we // write it to storage as auto resolved and then destroy it const data = { accountAddr: req.signAccountOp.accountOp.accountAddr, chainId: req.signAccountOp.accountOp.chainId, nonce: safeNonce, txnIds: [req.signAccountOp.accountOp.txnId] } const resolved = safeResolveIds.find( (txns) => txns.accountAddr === data.accountAddr && txns.chainId === data.chainId && txns.nonce === data.nonce ) if (!resolved) safeResolveIds.push(data) else resolved.txnIds.push(...data.txnIds) Unhandled error propagation restoreSafeUserRequest is a public controller method that awaits this.#safe.restoreTxnId([txnId]) without a try/catch or withStatus wrapper. If the storage write fails, the error propagates to the caller, violating the invariant that public controller methods must never propagate errors. It should use try/catch + this.emitError(...) or be wrapped in withStatus. async restoreSafeUserRequest(requestId: UserRequest['id']) { const request = this.userRequests.find( (r) => r.id === requestId && r.kind === 'calls' && r.meta.isSafeRejected ) as CallsUserRequest | undefined const txnId = request?.signAccountOp.accountOp.txnId const nonce = request ? getAccountOpNonce(request.signAccountOp.accountOp) : null if (!request || !txnId || nonce === null || nonce === undefined) return const currentNonce = this.#accounts.accountStates[request.meta.accountAddr]?.[request.meta.chainId.toString()] ?.nonce if (currentNonce !== undefined && nonce < currentNonce) return await this.#safe.restoreTxnId([txnId]) request.meta.isSafeRejected = false // on restore, simulate in the dashboard whatever is eligible void this.#performSimulation(this.userRequests, request) this.emitUpdate() } Premature simulation clear #performSimulation passes accountStateNonce from this.#accounts.accountStates, which can be undefined when the account state has not yet been loaded. When undefined, getSequentialSafeAccountOps returns an empty array (since no account op nonce can equal undefined), causing overrideSimulationResults to clear a valid dashboard simulation. This can happen transiently when a Safe request is added or restored before the account state finishes loading. async #performSimulation(requests: UserRequest[], curR: CallsUserRequest) { const accountStateNonce = this.#accounts.accountStates[curR.signAccountOp.account.addr]?.[ curR.signAccountOp.accountOp.chainId.toString() ]?.nonce const accountOps = curR.signAccountOp.account.safeCreation ? getSequentialSafeAccountOps(requests, curR, accountStateNonce) : [curR.signAccountOp.accountOp] // if no accountOps should be simulated, clear the results instead if (accountOps.length === 0) { void this.#portfolio.overrideSimulationResults(curR.signAccountOp.accountOp) return } void this.#portfolio.simulateAccountOp(accountOps) } Unhandled promise rejection #performSimulation is called with void in multiple locations and internally calls void this.#portfolio.simulateAccountOp(accountOps) and void this.#portfolio.overrideSimulationResults(...). If either of these async methods rejects (e.g., network failure in getOrFetchAccountOnChainState), the rejection is unhandled. This can crash the extension background process in strict environments. async #performSimulation(requests: UserRequest[], curR: CallsUserRequest) { const accountStateNonce = this.#accounts.accountStates[curR.signAccountOp.account.addr]?.[ curR.signAccountOp.accountOp.chainId.toString() ]?.nonce const accountOps = curR.signAccountOp.account.safeCreation ? getSequentialSafeAccountOps(requests, curR, accountStateNonce) : [curR.signAccountOp.accountOp] // if no accountOps should be simulated, clear the results instead if (accountOps.length === 0) { void this.#portfolio.overrideSimulationResults(curR.signAccountOp.accountOp) return } void this.#portfolio.simulateAccountOp(accountOps) }

[Oxbobby]

/review

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🔒 No security concerns identified

📝 TODO sections src/controllers/requests/requests.ts [943]: Double check simulation clearing condition when nextRequest is set in setCurrentRequest

⚡ Recommended focus areas for review Unhandled async errors In #performSimulation, the calls to this.#portfolio.simulateAccountOp(accountOps) and this.#portfolio.overrideSimulationResults(...) are prefixed with void, so they are not awaited. The surrounding try/catch only catches synchronous errors from the setup code (e.g., getSequentialSafeAccountOps), not async rejections from the portfolio methods. If simulateAccountOp rejects (e.g., due to an RPC failure), the rejection becomes an unhandled promise rejection rather than being caught and logged by the catch block. This gives a false sense of error safety. The fix is to await the calls instead of using void, or attach a .catch() handler. async #performSimulation(requests: UserRequest[], curR: CallsUserRequest) { try { const accountStateNonce = this.#accounts.accountStates[curR.signAccountOp.account.addr]?.[ curR.signAccountOp.accountOp.chainId.toString() ]?.nonce const accountOps = curR.signAccountOp.account.safeCreation ? getSequentialSafeAccountOps(requests, curR, accountStateNonce) : [curR.signAccountOp.accountOp] // if no accountOps should be simulated, clear the results instead if (accountOps.length === 0) { void this.#portfolio.overrideSimulationResults(curR.signAccountOp.accountOp) return } void this.#portfolio.simulateAccountOp(accountOps) } catch (e) { console.log('Failed to do #performSimulation', e) } Concurrent simulations #performSimulation is called with void from multiple locations, including inside the ids.forEach loop in removeUserRequests when rejecting multiple Safe requests. Each call triggers this.#portfolio.simulateAccountOp asynchronously without any guard against concurrent or out-of-order execution. If multiple Safe requests are rejected in a single removeUserRequests call, multiple simulateAccountOp calls run in parallel, each potentially calling this.storage.set() via updateSelectedAccount. This risks out-of-order state overwrites and parallel storage writes, which can corrupt persisted portfolio state. Consider debouncing, serializing, or aborting previous simulations before starting a new one. ids.forEach((id) => { const req = this.userRequests.find((uReq) => uReq.id === id) if (!req) return // A safe request could be rejected, but it also could be auto-resolved // when isReject is passed for a signed safe txn, we pause the signAccountOp // and allow the user to restore it at a later time. // If isReject is not passed, it means we're auto-removing an expired nonce // because another same nonce transaction has been broadcast if ( shouldRejectSafeRequests && req.kind === 'calls' && !!req.signAccountOp.account.safeCreation && !!req.signAccountOp.accountOp.txnId ) { req.meta.isSafeRejected = true req.dappPromises = [] req.signAccountOp.pause() safeRejectIds.push(req.signAccountOp.accountOp.txnId) // TODO: double check this // the simulation is getting cleared one level above removeUserRequests // however, it might start again if there's a nextRequest being set // in #setCurrentRequest. So double check if we have have a condition // when to fire this and when not to void this.#performSimulation(this.userRequests, req) return } Legacy data affected by unresolve The unresolve method filters out entries where nonce matches AND (accountAddr is undefined OR matches) AND (chainId is undefined OR matches). Legacy entries created before this PR lack accountAddr and chainId, so they are removed solely by nonce. If unresolve is called for account B on chain Y with nonce 131, any legacy entry with nonce 131 (potentially belonging to a different account/chain) will also be removed. This causes previously resolved transactions for unrelated accounts to reappear in the queue. The comment notes this is intentional for backward compatibility, but it could confuse users with legacy data until all entries are migrated. async unresolve(accountAddr: string, chainId: bigint, nonce: bigint) { // reset the counter so we could fetch immediately this.#updatedAt = undefined this.#automaticallyResolvedSafeTxns = this.#automaticallyResolvedSafeTxns.filter( (txns) => txns.nonce !== nonce || (!!txns.accountAddr && txns.accountAddr !== accountAddr) || (typeof txns.chainId !== 'undefined' && txns.chainId !== chainId) ) return this.#storage.set('automaticallyResolvedSafeTxns', this.#automaticallyResolvedSafeTxns)

[Oxbobby]

AI comments: 1 & 2 is basically how it should work from now on as those txns should be visible in the queue. I'm slightly adjusting 3

[JIOjosBG]

sorry if it sounds like i am shitting on this but i do not like the UI/UX

• too much info in that cramped space, it should be either minimal in the extension or fullscreen (i am leaning towards extension)

• lets not add extra button "Queue" on dash. I proposed having this in the activity page. It fits best there - this is future activity

• i do not understand the "Reject" and "Restore" buttons, neither should the user. If i press "Open" only that tx should be queued, not the rest with that nonce or with future nonce or on other chains

• not enough horizontal space now for humanizer. If we are going with extension screen we should 1) display nonce above the tx and 2) remove the hierarchy that eats horizontal space by making txns with sequential nonces be kinda sideways scrollable like we have in the screenshot below for requests in sign account op.

There is HUGE potential in this feature, we can blow people's hats off with simplicity, lets collaborate

[JIOjosBG]

One bug i had on my branch:

• if i click to simulate some N sequential txns if the simulation is successful the dashboard displays it properly
• if i then click to simulate any other sequence of txns which have one failing (for example expired swap) then the old simulation is persisted

For reference i merged this rework of the design

2026-08-12.18-47-20.mp4

[JIOjosBG]

We agreed that:

• in the dashboard for now we will display simulation of pending txns only if there is only one tx for that chain and we will not display it if there are competing and/or sequential txns on the same chain
• in the sign account op screen we will change the behavior of Reject to do what Sign later did. We will remove Sign later and we will change the wording or appearance of Reject
• in separate PR we can implement the Safe txns history from the Safe API

[Oxbobby]

/review

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🔒 No security concerns identified

✅ No TODO sections

⚡ Recommended focus areas for review Rejected txns no longer filtered #filterOutHidden previously filtered both rejected and automatically-resolved Safe transactions from the txns array returned by fetchPending. The new code only filters messages — txns: pending[chainId]!.txns passes all transactions through unfiltered. When fetchSafeTxns runs again, toCallsUserRequest will build new user requests for previously rejected transactions, and since they were already spliced out of userRequests by removeUserRequests, #createOrUpdateCallsUserRequest will treat them as new. This causes transactions the user explicitly rejected to reappear as active signing requests. The PR description states that rejected transactions should be marked with isSafeRejected, but no code setting that flag is visible in the diff — if the marking logic lives outside the shown hunks this may be a non-issue, but from the diff alone there is no mechanism to prevent or identify the reappearing rejected requests. #filterOutHidden(pending: SafeResults, safeAddr: string): SafeResults { const hiddenMessages = [...this.#rejectedSafeTxns] return Object.assign( {}, ...Object.keys(pending).map((chainId) => { const state = this.#accounts.accountStates[safeAddr]?.[chainId] return { [chainId]: { txns: pending[chainId]!.txns, messages: pending[chainId]!.messages.filter((m) => { return ( // filter out rejected msgs by the user !hiddenMessages.includes(this.getMessageId(m)) && !hiddenMessages.includes( `${this.getMessageId(m)}-${new Date(m.created).getTime()}` ) && // and those that the user cannot sign (state?.threshold || 0) > m.confirmations.length ) }) } } }) ) Automatically resolved txns not hidden The #automaticallyResolvedSafeTxns field and the resolveTxnId/unresolve methods were removed, and #filterOutHidden no longer filters resolved txns from the txns array. The PR description states "Automatically resolved transactions remain hidden from future fetches," but no replacement hiding mechanism is visible in the diff. When a Safe transaction is broadcast, competing same-nonce requests are removed from the queue via removeUserRequests, but they are no longer persisted as automatically resolved. On the next fetchPending call, these still-pending Safe transactions will reappear from Safe Global and be rebuilt as new active requests, unless they have since been executed on-chain. This may be handled by code outside the diff, but from what is visible there is no guard against their return. #filterOutHidden(pending: SafeResults, safeAddr: string): SafeResults { const hiddenMessages = [...this.#rejectedSafeTxns] return Object.assign( {}, ...Object.keys(pending).map((chainId) => { const state = this.#accounts.accountStates[safeAddr]?.[chainId] return { [chainId]: { txns: pending[chainId]!.txns, messages: pending[chainId]!.messages.filter((m) => { return ( // filter out rejected msgs by the user !hiddenMessages.includes(this.getMessageId(m)) && !hiddenMessages.includes( `${this.getMessageId(m)}-${new Date(m.created).getTime()}` ) && // and those that the user cannot sign (state?.threshold || 0) > m.confirmations.length ) }) } } }) )

[Oxbobby]

The AI reviews comments are irrelevant as we changed the logic to always display transactions in the queue