Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 14 additions & 7 deletions test/integration/uexecutor/execute_inbound_gas_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,16 @@ func TestInboundGas(t *testing.T) {
"revert outbound amount must match inbound amount")
require.Equal(t, inbound.AssetAddr, ob.ExternalAssetAddr,
"revert outbound asset must match inbound asset")
require.Equal(t, uexecutortypes.Status_PENDING, ob.OutboundStatus,
"revert outbound should start in PENDING status")

// Gas fields are populated from UniversalCore if chain meta is set.
// In test env without VoteChainMeta, they may be zero/empty — that's OK,
// the outbound is still created (graceful degradation).
// The UniversalCore stub deployed by the integration harness cannot
// serve getOutboundTxGasAndFees, so the revert's gas metadata is
// unresolvable here and the outbound is recorded ABORTED rather than
// queued for a signature it could never receive. The resolvable
// (PENDING) path is covered by
// x/uexecutor/keeper/build_revert_outbound_test.go.
require.Equal(t, uexecutortypes.Status_ABORTED, ob.OutboundStatus,
"a revert with unresolvable gas metadata must be ABORTED, not PENDING")
require.NotEmpty(t, ob.AbortReason, "ABORTED revert must carry a reason")
requireNotQueuedForSigning(t, chainApp, ctx, ob.Id)
// When chain meta IS set, these will be populated.
break
}
Expand Down Expand Up @@ -464,7 +468,10 @@ func TestInboundGas(t *testing.T) {
if ob.TxType == uexecutortypes.TxType_INBOUND_REVERT {
foundRevert = true
require.Equal(t, inbound.SourceChain, ob.DestinationChain)
require.Equal(t, uexecutortypes.Status_PENDING, ob.OutboundStatus)
// Gas metadata is unresolvable against the harness's UniversalCore stub,
// so the revert is recorded ABORTED instead of entering the signing queue.
require.Equal(t, uexecutortypes.Status_ABORTED, ob.OutboundStatus)
requireNotQueuedForSigning(t, chainApp, ctx, ob.Id)
break
}
}
Expand Down
158 changes: 158 additions & 0 deletions test/integration/uexecutor/inbound_revert_abort_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,158 @@
package integrationtest

import (
"math/big"
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/ethereum/go-ethereum/common"
"github.com/stretchr/testify/require"

"github.com/pushchain/push-chain-node/app"
utils "github.com/pushchain/push-chain-node/test/utils"
uexecutortypes "github.com/pushchain/push-chain-node/x/uexecutor/types"
)

// Regression coverage for F-2026-18823.
//
// buildRevertOutbound used to fail open: when it could not resolve the revert's
// gas metadata it logged "proceeding without gas fields" and returned the
// outbound anyway, still marked PENDING. attachOutboundsToUtx then indexed it
// into PendingOutbounds unconditionally, where the universal validators refused
// to sign it ("gas price is zero or missing"). The row could never leave the
// queue: no ballot forms for an unsignable outbound and there is no admin abort
// for outbounds. Worse, non-CEA rescue was gated on an INBOUND_REVERT having
// reached REVERTED, so the user had no recovery route either.
//
// The revert is now recorded ABORTED with a reason, kept off the signing queue,
// and accepted by the rescue gate.
//
// NOTE ON THIS ENVIRONMENT: the UniversalCore contract deployed by the test
// harness cannot serve getOutboundTxGasAndFees (its PRC20 stub has no
// SOURCE_CHAIN_NAMESPACE), so every INBOUND_REVERT built here takes the abort
// path. That makes the failure realistic end-to-end but means the resolvable
// path cannot be exercised at this level; it is covered by
// x/uexecutor/keeper/build_revert_outbound_test.go, which drives the same
// function with the gas lookup mocked both ways.

// requireNotQueuedForSigning asserts that an outbound was never indexed into
// PendingOutbounds, i.e. it will not be picked up for TSS signing.
func requireNotQueuedForSigning(t *testing.T, chainApp *app.ChainApp, ctx sdk.Context, outboundId string) {
t.Helper()
has, err := chainApp.UexecutorKeeper.PendingOutbounds.Has(ctx, outboundId)
require.NoError(t, err)
require.False(t, has,
"outbound %s must not be indexed in PendingOutbounds: it can never be signed and nothing would ever remove it", outboundId)
}

// findInboundRevert returns the INBOUND_REVERT outbound on a UTX, if any.
func findInboundRevert(utx uexecutortypes.UniversalTx) *uexecutortypes.OutboundTx {
for _, ob := range utx.OutboundTx {
if ob != nil && ob.TxType == uexecutortypes.TxType_INBOUND_REVERT {
return ob
}
}
return nil
}

// driveNonCEAInboundToAbortedRevert votes a non-CEA FUNDS inbound with an empty
// recipient to quorum. Execution validation rejects it, so an INBOUND_REVERT is
// built — and since the harness cannot serve gas metadata, that revert aborts.
//
// The token/chain config is deliberately left registered so the failure is the
// gas lookup alone; the PRC20-not-found variant is covered in
// vote_inbound_validation_test.go.
func driveNonCEAInboundToAbortedRevert(t *testing.T, txHash string) (*app.ChainApp, sdk.Context, string) {
t.Helper()

chainApp, ctx, vals, inbound, coreVals := setupInboundBridgeTest(t, 4)
inbound.TxHash = txHash
inbound.IsCEA = false
inbound.Recipient = "" // FUNDS requires a recipient — fails ValidateForExecution post-quorum

for i := 0; i < 3; i++ {
valAddr, err := sdk.ValAddressFromBech32(coreVals[i].OperatorAddress)
require.NoError(t, err)
require.NoError(t, utils.ExecVoteInbound(t, ctx, chainApp, vals[i], sdk.AccAddress(valAddr).String(), inbound))
}

return chainApp, ctx, uexecutortypes.GetInboundUniversalTxKey(*inbound)
}

// TestInboundRevert_UnresolvableGasMetadata_AbortsInsteadOfQueueing is the
// headline regression test: the revert must be recorded ABORTED with a reason
// and must never reach PendingOutbounds.
func TestInboundRevert_UnresolvableGasMetadata_AbortsInsteadOfQueueing(t *testing.T) {
chainApp, ctx, utxId := driveNonCEAInboundToAbortedRevert(t, "0xabortrevert01")

utx, found, err := chainApp.UexecutorKeeper.GetUniversalTx(ctx, utxId)
require.NoError(t, err)
require.True(t, found, "UTX must exist after quorum")

revert := findInboundRevert(utx)
require.NotNil(t, revert, "a failed non-CEA inbound must still record an INBOUND_REVERT attempt")

require.Equal(t, uexecutortypes.Status_ABORTED, revert.OutboundStatus,
"a revert whose gas metadata could not be resolved must be ABORTED, never PENDING")
require.NotEmpty(t, revert.AbortReason, "the abort reason must say why the revert could not be built")
require.Contains(t, revert.AbortReason, "gas fee info",
"the reason must name the lookup that failed")

// Fail-closed: the gas fields stay empty rather than being half-written.
require.Empty(t, revert.GasToken)
require.Empty(t, revert.GasFee)
require.Empty(t, revert.GasPrice)
require.Empty(t, revert.GasLimit)

requireNotQueuedForSigning(t, chainApp, ctx, revert.Id)

// The whole queue stays clean, not just this id.
err = chainApp.UexecutorKeeper.PendingOutbounds.Walk(ctx, nil, func(id string, _ uexecutortypes.PendingOutboundEntry) (bool, error) {
t.Fatalf("PendingOutbounds must be empty, found %s", id)
return true, nil
})
require.NoError(t, err)
}

// TestInboundRevert_AbortedRevert_UnlocksRescue proves the other half of the
// fix: skipping the queue is not enough on its own, because non-CEA rescue used
// to require a REVERTED inbound-revert. An ABORTED one must now be accepted, or
// the user is left with a clean queue and no way out.
func TestInboundRevert_AbortedRevert_UnlocksRescue(t *testing.T) {
chainApp, ctx, utxId := driveNonCEAInboundToAbortedRevert(t, "0xabortrevert02")

utx, found, err := chainApp.UexecutorKeeper.GetUniversalTx(ctx, utxId)
require.NoError(t, err)
require.True(t, found)
revert := findInboundRevert(utx)
require.NotNil(t, revert)
require.Equal(t, uexecutortypes.Status_ABORTED, revert.OutboundStatus,
"precondition: the revert must have aborted for this test to mean anything")

prc20Addr := utils.GetDefaultAddresses().PRC20USDCAddr
senderAddr := common.HexToAddress(utils.GetDefaultAddresses().DefaultTestAddr)
log := buildRescueFundsLog(t, utxId, prc20Addr, senderAddr,
"eip155", big.NewInt(333), big.NewInt(1_000_000_000), big.NewInt(200_000))

err = chainApp.UexecutorKeeper.AttachRescueOutboundFromReceipt(
ctx,
makeRescueReceipt(t, "0xrescueafterabort", log),
uexecutortypes.PCTx{TxHash: "0xrescueafterabort", Status: "SUCCESS"},
)
require.NoError(t, err, "rescue must be accepted when the auto-revert aborted; the funds never came back")

utx, _, err = chainApp.UexecutorKeeper.GetUniversalTx(ctx, utxId)
require.NoError(t, err)

rescue := findRescueOutbound(utx)
require.NotNil(t, rescue, "a RESCUE_FUNDS outbound must be attached")
require.Equal(t, uexecutortypes.Status_PENDING, rescue.OutboundStatus,
"the rescue itself is signable and must be queued")
require.Equal(t, "333", rescue.GasFee)

// The rescue is queued; the aborted revert still is not.
has, err := chainApp.UexecutorKeeper.PendingOutbounds.Has(ctx, rescue.Id)
require.NoError(t, err)
require.True(t, has, "the rescue outbound must be indexed for UV pickup")
requireNotQueuedForSigning(t, chainApp, ctx, revert.Id)
}
4 changes: 2 additions & 2 deletions test/integration/uexecutor/rescue_funds_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ func TestRescueFunds(t *testing.T) {
"eip155", big.NewInt(111), big.NewInt(1_000_000_000), big.NewInt(200_000))
err := chainApp.UexecutorKeeper.AttachRescueOutboundFromReceipt(ctx, makeRescueReceipt(t, "0xrescuetx03", log), uexecutortypes.PCTx{TxHash: "0xrescuetx03", Status: "SUCCESS"})
require.Error(t, err)
require.Contains(t, err.Error(), "no reverted inbound-revert outbound")
require.Contains(t, err.Error(), "no reverted or aborted inbound-revert outbound")
})

t.Run("rescue is rejected for non-CEA inbound when auto-revert is PENDING", func(t *testing.T) {
Expand Down Expand Up @@ -255,7 +255,7 @@ func TestRescueFunds(t *testing.T) {
"eip155", big.NewInt(111), big.NewInt(1_000_000_000), big.NewInt(200_000))
err = chainApp.UexecutorKeeper.AttachRescueOutboundFromReceipt(ctx, makeRescueReceipt(t, "0xrescuetx03b", log), uexecutortypes.PCTx{TxHash: "0xrescuetx03b", Status: "SUCCESS"})
require.Error(t, err)
require.Contains(t, err.Error(), "no reverted inbound-revert outbound")
require.Contains(t, err.Error(), "no reverted or aborted inbound-revert outbound")
})

t.Run("rescue succeeds for non-CEA inbound with reverted auto-revert", func(t *testing.T) {
Expand Down
20 changes: 15 additions & 5 deletions test/integration/uexecutor/revert_stuck_inbound_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,15 @@ func TestRevertStuckInbound_HappyPath_ExpiredBallot_CreatesRevertOutbound(t *tes
require.Equal(t, uexecutortypes.GetOutboundRevertId(inbound.SourceChain, inbound.TxHash, inbound.LogIndex), ob.Id,
"outbound id must follow the canonical revert-id format")
require.Equal(t, uexecutortypes.TxType_INBOUND_REVERT, ob.TxType, "outbound type must be INBOUND_REVERT")
require.Equal(t, uexecutortypes.Status_PENDING, ob.OutboundStatus, "outbound must start PENDING so UVs sign it")
// The harness's UniversalCore stub cannot serve getOutboundTxGasAndFees, so the
// revert's gas metadata is unresolvable and it is recorded ABORTED rather than
// queued for a signature it could never receive. The admin message still reports
// the outbound it created, and the UTX becomes eligible for RESCUE_FUNDS. The
// resolvable (PENDING) path is covered by
// x/uexecutor/keeper/build_revert_outbound_test.go.
require.Equal(t, uexecutortypes.Status_ABORTED, ob.OutboundStatus,
"a revert with unresolvable gas metadata must be ABORTED, not PENDING")
require.NotEmpty(t, ob.AbortReason, "ABORTED revert must record why it could not be built")
require.Equal(t, inbound.SourceChain, ob.DestinationChain, "revert goes back to the source chain")
require.Equal(t, inbound.RevertInstructions.FundRecipient, ob.Recipient,
"recipient must use RevertInstructions.FundRecipient when set")
Expand All @@ -134,10 +142,12 @@ func TestRevertStuckInbound_HappyPath_ExpiredBallot_CreatesRevertOutbound(t *tes
require.Equal(t, chainutils.LenientCanonicalizeEVMAddress(inbound.Sender), ob.Sender, "sender field carries original depositor")

// --- PendingOutbounds index assertions ---
pending, err := chainApp.UexecutorKeeper.PendingOutbounds.Get(ctx, ob.Id)
require.NoError(t, err, "revert outbound must be indexed in PendingOutbounds for UV pickup")
require.Equal(t, ob.Id, pending.OutboundId)
require.Equal(t, utx.Id, pending.UniversalTxId)
// An ABORTED revert must stay out of the signing queue: no ballot can ever form
// for it and there is no admin abort for outbounds, so an indexed row would be
// permanently stuck.
has, err := chainApp.UexecutorKeeper.PendingOutbounds.Has(ctx, ob.Id)
require.NoError(t, err)
require.False(t, has, "an ABORTED revert must not be indexed in PendingOutbounds")
}

// TestRevertStuckInbound_RecipientFallback_UsesSender covers the case where
Expand Down
13 changes: 11 additions & 2 deletions test/integration/uexecutor/vote_inbound_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,11 @@ func TestVoteInboundValidation(t *testing.T) {
foundRevert = true
require.Equal(t, inbound.SourceChain, ob.DestinationChain)
require.Equal(t, inbound.Amount, ob.Amount)
require.Equal(t, uexecutortypes.Status_PENDING, ob.OutboundStatus)
// The harness's UniversalCore stub cannot serve gas metadata, so the
// revert is unsignable and is recorded ABORTED instead of queued.
require.Equal(t, uexecutortypes.Status_ABORTED, ob.OutboundStatus)
require.NotEmpty(t, ob.AbortReason)
requireNotQueuedForSigning(t, chainApp, ctx, ob.Id)
break
}
}
Expand Down Expand Up @@ -356,7 +360,12 @@ func TestVoteInboundValidation(t *testing.T) {
require.Equal(t, inbound.SourceChain, ob.DestinationChain)
require.Equal(t, inbound.Amount, ob.Amount)
require.Equal(t, inbound.AssetAddr, ob.ExternalAssetAddr)
require.Equal(t, uexecutortypes.Status_PENDING, ob.OutboundStatus)
// The token config was removed above, so the revert cannot resolve the
// PRC20 it needs for gas metadata. It is recorded ABORTED with the
// reason instead of being queued as an unsignable PENDING row.
require.Equal(t, uexecutortypes.Status_ABORTED, ob.OutboundStatus)
require.Contains(t, ob.AbortReason, "failed to resolve PRC20")
requireNotQueuedForSigning(t, chainApp, ctx, ob.Id)
break
}
}
Expand Down
16 changes: 14 additions & 2 deletions x/uexecutor/keeper/admin_revert.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,20 @@ func (k Keeper) RevertStuckInbound(ctx context.Context, inbound types.Inbound) (
return "", "", fmt.Errorf("failed to create utx for revert: %w", cErr)
}

revertOutbound := k.buildRevertOutbound(sdkCtx, &inbound)
revertOutbound, buildErr := k.buildRevertOutbound(sdkCtx, &inbound)
if revertOutbound == nil {
return "", "", fmt.Errorf("failed to build revert outbound for inbound %s", universalTxKey)
return "", "", fmt.Errorf("failed to build revert outbound for inbound %s: %w", universalTxKey, buildErr)
}
if buildErr != nil {
// Gas metadata was unresolvable, so the revert is recorded ABORTED instead of
// entering the signing queue. It is still attached: the attempt stays auditable
// and it makes the UTX eligible for RESCUE_FUNDS, which is the remaining route
// back to the user.
k.Logger().Error("admin revert: revert outbound recorded without gas metadata",
"utx_id", universalTxKey,
"outbound_id", revertOutbound.Id,
"error", buildErr.Error(),
)
}

if attachErr := k.attachOutboundsToUtx(sdkCtx, universalTxKey, []*types.OutboundTx{revertOutbound}, "admin revert: stuck ballot expired"); attachErr != nil {
Expand All @@ -82,6 +93,7 @@ func (k Keeper) RevertStuckInbound(ctx context.Context, inbound types.Inbound) (
k.Logger().Info("admin revert: inbound revert outbound created",
"utx_id", universalTxKey,
"outbound_id", revertOutbound.Id,
"status", revertOutbound.OutboundStatus.String(),
"source_chain", inbound.SourceChain,
"recipient", revertOutbound.Recipient,
"amount", revertOutbound.Amount,
Expand Down
Loading
Loading