-
Notifications
You must be signed in to change notification settings - Fork 5
fix: mark stake transactions correctly when they land more than one block after broadcast #278
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -44,7 +44,7 @@ export async function ExecuteTransaction(args: TransactionArgs) { | |
| updateTransaction, | ||
| executeTransaction, | ||
| getBlockHeight, | ||
| verifyTransaction, | ||
| checkSupplierOnChain, | ||
| createNewNodesFromTransaction, | ||
| notifyProviderOfStakedAddresses, | ||
| notifyProviderOfFailedStakes, | ||
|
|
@@ -57,6 +57,18 @@ export async function ExecuteTransaction(args: TransactionArgs) { | |
| }, | ||
| }); | ||
|
|
||
| // verifyTransaction polls the chain on its own retry schedule: one attempt per block | ||
| // (pocket block time ≈ 1 min) up to TX_EXPIRATION_BLOCKS, matching the on-chain | ||
| // mempool expiration window. Throws retriable `TX_NOT_FOUND` until the tx lands. | ||
| const { verifyTransaction } = proxyActivities<ReturnType<typeof delegatorActivities>>({ | ||
| startToCloseTimeout: "30s", | ||
| retry: { | ||
| initialInterval: "60s", | ||
| backoffCoefficient: 1, | ||
| maximumAttempts: TX_EXPIRATION_BLOCKS, | ||
| }, | ||
| }); | ||
|
|
||
| const transaction = await getTransaction(transactionId); | ||
|
|
||
| if (transaction.status !== TransactionStatus.Pending) { | ||
|
|
@@ -127,20 +139,58 @@ export async function ExecuteTransaction(args: TransactionArgs) { | |
| await waitForNextBlock(txHeight); | ||
| } | ||
|
|
||
| const [success, code, gasUsed] = await verifyTransaction( | ||
| result?.transactionHash || transaction.hash!, | ||
| transaction.executionHeight || txHeight, | ||
| extractOperatorAddress(transaction), | ||
| ); | ||
| const txHash = result?.transactionHash || transaction.hash!; | ||
| const baseHeight = transaction.executionHeight || txHeight; | ||
| const operatorAddress = extractOperatorAddress(transaction); | ||
|
|
||
| const txStatus = success ? TransactionStatus.Success : TransactionStatus.Failure; | ||
| let success = false; | ||
| let code = -1; | ||
| let gasUsed = '0'; | ||
| let txFoundOnChain = false; | ||
| let supplierFallbackHit = false; | ||
|
|
||
| try { | ||
| // Retries are driven by Temporal's activity retry policy — one attempt per block | ||
| // up to TX_EXPIRATION_BLOCKS. A found tx returns the tuple; a missing tx throws | ||
| // retriable TX_NOT_FOUND until the policy is exhausted. | ||
| [success, code, gasUsed] = await verifyTransaction(txHash, baseHeight); | ||
| txFoundOnChain = true; | ||
| } catch { | ||
| if (operatorAddress) { | ||
| try { | ||
| const supplierExists = await checkSupplierOnChain(operatorAddress); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Failure case: another operator (or the same owner via a parallel route) stakes the same operator address while our original tx was lost or failed. Tier 4 sees a supplier exists with that operator and returns Minimum: have |
||
| if (supplierExists) { | ||
| success = true; | ||
| code = 0; | ||
| gasUsed = '0'; | ||
| supplierFallbackHit = true; | ||
| } | ||
| } catch { | ||
| // supplier check also failed — tx stays marked as Failure (success stays false) | ||
| } | ||
| } | ||
| } | ||
|
|
||
| const txStatus = success ? TransactionStatus.Success : TransactionStatus.Failure; | ||
| const verificationHeight = await getBlockHeight(); | ||
|
|
||
| let verificationLog: string | undefined; | ||
| if (txFoundOnChain) { | ||
| if (code !== 0) { | ||
| verificationLog = `verification failed with code ${code}`; | ||
| } | ||
| } else if (supplierFallbackHit) { | ||
| verificationLog = 'verified via supplier state fallback (tx hash not found)'; | ||
| } else { | ||
| verificationLog = `tx not found on-chain after ${TX_EXPIRATION_BLOCKS} retries (baseHeight=${baseHeight})`; | ||
| } | ||
|
|
||
| await updateTransaction(transactionId, { | ||
| status: txStatus, | ||
| verificationHeight, | ||
| consumedFee: Number(gasUsed || 0), | ||
| code, | ||
| log: verificationLog, | ||
| }); | ||
|
|
||
| if (transaction.type === TransactionType.Stake) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
txHeightpassed here comes from line 78 (const txHeight = await getBlockHeight()), which is captured BEFOREexecuteTransaction. If the chain advances during the broadcast — perfectly possible under load with variable block time —txHeightis stale.waitForNextBlock(txHeight)checkscurrentHeight >= txHeight + 1. Since we already advanced, it returns immediately without waiting — exactly the wait the rest of the fix relies on.verifyTransactionthen goes to look for the tx in the same block where it was included, where the tx_index probably hasn't indexed it yet.The retries with 60s backoff eventually recover, but we're burning minutes of overhead that
waitForNextBlockwas meant to avoid.Fix: re-capture the height post-broadcast before calling
waitForNextBlock, or wait fortxHeight + 2to have a one-block margin.