Skip to content

feat: Transaction manager for centralised txn submission#3385

Open
pxrl wants to merge 2 commits into
masterfrom
pxrl/txnMgr
Open

feat: Transaction manager for centralised txn submission#3385
pxrl wants to merge 2 commits into
masterfrom
pxrl/txnMgr

Conversation

@pxrl
Copy link
Copy Markdown
Collaborator

@pxrl pxrl commented May 12, 2026

This permits multiple bots to share a single EOA with centralised transaction lifecycle management, nonce tracking etc. This is experimental but should permit more app-level features to be added for a given EOA without leading to increased congestion on the chain.

This permits multiple bots to share a single EOA with centralised
transaction lifecycle management, nonce tracking etc. This is
experimental but should permit more app-level features to be added for a
given EOA without leading to increased congestion on the chain.
Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 821f0e45cf

ℹ️ 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".

Comment on lines +323 to +326
let response: TransactionResponse;
try {
response = await backend.submit(txn);
} catch (error) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Preserve ensureConfirmation semantics in redis submit path

The redis branch bypasses _submit, so transactions with ensureConfirmation: true now return immediately after ACK instead of waiting for on-chain confirmation. This is a behavior regression versus the RPC path and can break sequencing flows that rely on confirmation before continuing (e.g. approval-then-action patterns), because callers that previously depended on submit() blocking for these transactions will now race ahead.

Useful? React with 👍 / 👎.

Comment on lines +226 to +227
const [r] = await this.txnClient.submit(req.chainId, [augTxn]);
response = r;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Emit ACK failure when manager submit returns no response

TransactionClient.submit() can resolve with an empty array on submission failure (it logs and returns partial results), but this code destructures [r] and returns undefined without publishing an ACK failure. In that case processRequest exits as if a failure ACK was already emitted, and the caller-side backend waits indefinitely on ackD.promise for this request id.

Useful? React with 👍 / 👎.

Comment on lines +233 to +235
this.subscribingPromises.set(key, pending);
}
await pending;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear failed subscription promises to allow backend recovery

The first subscription attempt for a (chainId,eoa) key is cached in subscribingPromises, but the entry is never removed. If sub() fails once (for example during a transient Redis disconnect), the cached rejected promise is reused forever, so all later submits for that key fail immediately even after Redis recovers.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant