Skip to content

Add TransactionManager to handle stalled tx#767

Open
evgeny-stakewise wants to merge 2 commits into
masterfrom
tx-replace-pending
Open

Add TransactionManager to handle stalled tx#767
evgeny-stakewise wants to merge 2 commits into
masterfrom
tx-replace-pending

Conversation

@evgeny-stakewise

Copy link
Copy Markdown
Contributor

Summary

Routes all wallet transactions through a single TransactionManager.transact(...) path and removes transaction_gas_wrapper. The manager owns submit + receipt-wait under a lock (one in-flight tx at a time), reuses the lowest unconfirmed nonce, and replaces a stuck pending tx with bumped fees instead of an in-call retry loop.

What changed

  • transact(tx_function, tx_params=None, high_priority=False) -> TxReceipt | None: serializes submit and the receipt wait so exactly one wallet tx is in flight at a time; returns the receipt (or None on revert).
  • Two gas strategies:
    • high_priority=True (e.g. validator registration) skips the default-gas attempts and submits high-priority fees straight away.
    • high_priority=False keeps the existing default-gas attempts loop, escalating to high-priority fees on FeeTooLow.
  • No in-call fee-bump loop. A run submits once; if it doesn't confirm within execution_transaction_timeout the task retries on its next run, where a pending tx is detected and its fees bumped (max(high_priority, bump(prev)), capped at max_fee_per_gas).
  • Deleted transaction_gas_wrapper; migrated every caller (validators register/fund/consolidate, withdrawals, exits, harvest, reward splitter, meta vault) plus the tx_aggregate / deposit_to_sub_vaults wrappers, which now return TxReceipt | None.

Behavior note

Because the manager now owns the receipt wait, a receipt timeout that previously propagated out of a caller is now caught by the caller's existing except Exception and returns None — still retried on the next task run.

@evgeny-stakewise evgeny-stakewise changed the title Integrate transaction_gas_wrapper into TransactionManager Add TransactionManager to handle stalled tx Jun 24, 2026
Comment thread src/common/transaction.py

# forget gas records for nonces that have already been mined (nonce
# `latest_nonce` itself may still be pending, so keep it)
self._nonce_to_tx_params = {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Optional: complicated one-liner, can be exracted into _update_nonce... func

Comment thread src/common/transaction.py
self.priority_fee_per_gas = self._bump(self.priority_fee_per_gas)
self._cap()

def _cap(self) -> None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

move private funcs in the end of the class

Comment thread src/common/transaction.py
max_fee_per_gas=self.max_fee_per_gas,
)

def tx_params(self) -> TxParams:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Optional: Use @Property or smth like get_tx_params

gas_manager = build_gas_manager()
tx_params = await gas_manager.get_high_priority_tx_params()
tx = await vault_contract.functions.multicall(calls).transact(tx_params)
tx_receipt = await tx_manager.transact(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

estimate_gas will pass, but tx can fail if there is a lock

Comment thread src/common/transaction.py
return None

@staticmethod
async def _wait_for_receipt(tx_hash: HexBytes) -> TxReceipt | None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

catch from claude:

TimeExhausted handling is inconsistent and partly defeats the 
  design (transaction.py:194-201)
  _wait_for_receipt doesn't catch TimeExhausted, so timeouts raise
  instead of returning None. The PR's "stop and let next run replace
  it" story depends on returning None. Behavior now diverges by call
  site: exits/validators/withdrawals swallow it (except Exception),
  but harvest/execution.py:23 (catches only ValueError, 
  ContractLogicError) and meta_vault/reward_splitter (no try)
  propagate it as a task-level crash. Fix centrally: catch
  TimeExhausted in _wait_for_receipt, return None, log "pending, will
  replace next run."

Comment thread src/common/transaction.py
# an earlier transaction is stuck at latest_nonce - replace it instead of
# queuing a new one behind it (skip the default-gas attempts)
logger.info('Found pending transaction at nonce %d, replacing it', latest_nonce)
tx_hash = await self._submit_high_priority(tx_function, tx_params, latest_nonce)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

catch from claude: Looks like node can revert tx with same params
At-cap replacement can't bump → "replacement underpriced" wedge
(transaction.py:47-56,152-171)
Once a pending tx's fee_per_gas is at max_fee_per_gas, bump()→_cap()
leaves it unchanged, so the replacement is broadcast with identical
maxFeePerGas and rejected (-32000 underpriced ValueError). That
error isn't caught in submit_high_priority, so it surfaces as a
generic failure and the nonce stays stuck until base fee drops. The
docstring's "so it converges" is misleading. Detect this case and
log a clear warning.

Why the node rejects it

EIP-1559 replacement rules in geth/Nethermind require a same-nonce
replacement to raise both maxFeePerGas and maxPriorityFeePerGas by
at least the configured price bump (default 10%). A 0% increase is
rejected with replacement transaction underpriced — a ValueError,
JSON-RPC code -32000

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.

3 participants