Skip to content

fix(satp-hermes): return missing promise in rollback strategies#4148

Open
atharrva01 wants to merge 1 commit intohyperledger-cacti:mainfrom
atharrva01:fix/satp-rollback-missing-return
Open

fix(satp-hermes): return missing promise in rollback strategies#4148
atharrva01 wants to merge 1 commit intohyperledger-cacti:mainfrom
atharrva01:fix/satp-rollback-missing-return

Conversation

@atharrva01
Copy link

I found a critical async bug across all four SATP rollback strategy files , every context.with() call was missing a return statement, silently discarding the promise and letting rollback methods resolve immediately without doing any actual work.

The Problem

The execute() method awaits handleClientSideRollback(), but since the inner promise was dropped, it resolved instantly , with an empty rollbackLogEntries array. The crash manager saw no failures and marked the rollback "COMPLETED" before a single ledger operation had run.

// Before -  promise is discarded
context.with(ctx, async () => { await bridge.mintAsset(asset); });

// After - promise is properly awaited
return context.with(ctx, async () => { await bridge.mintAsset(asset); });

Why It Matters

This broke the entire crash recovery path. A Stage 3 failure should mint assets back on the source chain and burn them on the destination, but neither happened. In practice, this could mean permanently lost or duplicated assets, with bridge errors silently swallowed as unhandled rejections.

The Fix

One word , return` , added at 7 call sites across 4 files. No logic changes, no refactoring. Rollbacks now complete fully before status is evaluated, failed rollbacks surface correctly, and cross-ledger atomicity during crash recovery actually holds.

Signed-off-by: atharrva01 <atharvaborade568@gmail.com>
@atharrva01
Copy link
Author

hi @AndreAugusto11 @RafaelAPB this PR fixes missing return before context.with() calls in all four SATP rollback strategies promises were being silently discarded, causing rollbacks to appear completed before any ledger operations actually ran.

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