Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
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
26 changes: 26 additions & 0 deletions zetaclient/chains/sui/signer/signer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,32 @@ func TestSigner(t *testing.T) {
})
}

func TestSigner_BroadcastWithdrawalWithFallback(t *testing.T) {
t.Run("cancels on generic withdraw build error", func(t *testing.T) {
ts := newTestSuite(t)

withdrawTxBuilder := func(context.Context) (models.TxnMetaData, string, error) {
return models.TxnMetaData{}, "", fmt.Errorf("invalid type argument")
}

cancelTxBytes := base64.StdEncoding.EncodeToString([]byte("cancel_tx_bytes"))
cancelTxBuilder := func(context.Context) (models.TxnMetaData, string, error) {
return models.TxnMetaData{TxBytes: cancelTxBytes}, "cancel_signature", nil
}

const cancelDigest = "0xCancelTransactionBlockDigest"
ts.MockExec(func(req models.SuiExecuteTransactionBlockRequest) {
require.Equal(t, cancelTxBytes, req.TxBytes)
require.Equal(t, []string{"cancel_signature"}, req.Signature)
}, cancelDigest)

digest, err := ts.Signer.broadcastWithdrawalWithFallback(ts.Ctx, withdrawTxBuilder, cancelTxBuilder)

require.NoError(t, err)
require.Equal(t, cancelDigest, digest)
})
}

type testSuite struct {
t *testing.T
Ctx context.Context
Expand Down
3 changes: 2 additions & 1 deletion zetaclient/chains/sui/signer/signer_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -282,7 +282,8 @@ func (s *Signer) broadcastWithdrawalWithFallback(
logger.Info().Err(err).Msg("cancelling tx due to wrong object ownership")
return s.broadcastCancelTx(ctx, cancelTxBuilder)
case err != nil:
return "", errors.Wrap(err, "unable to build withdraw tx")
logger.Info().Err(err).Msg("cancelling tx due to withdraw tx build error")
return s.broadcastCancelTx(ctx, cancelTxBuilder)

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 Transient build errors now trigger irreversible cancel

The generic err != nil branch now calls broadcastCancelTx, which broadcasts an increase_nonce transaction that permanently consumes the withdrawal nonce. Any transient error during buildWithdrawal — for example a MoveCall RPC timeout, a temporary getWithdrawCapIDCached miss, or a getWithdrawAndCallObjectRefs network failure — will now permanently cancel the withdrawal instead of bubbling the error back to the scheduler for a retry. The existing ErrInvalidPayload / ErrObjectOwnership branches already cover the known deterministic cases; the remaining errors are a mix of deterministic (e.g. invalid gas price, unsupported coin type) and genuinely transient (RPC connectivity). Consider distinguishing the two categories — or at minimum wrapping the specific Sui-type-argument error as a new sentinel — so that transient failures can still surface as retryable errors.

Prompt To Fix With AI
This is a comment left during a code review.
Path: zetaclient/chains/sui/signer/signer_tx.go
Line: 284-286

Comment:
**Transient build errors now trigger irreversible cancel**

The generic `err != nil` branch now calls `broadcastCancelTx`, which broadcasts an `increase_nonce` transaction that permanently consumes the withdrawal nonce. Any transient error during `buildWithdrawal` — for example a `MoveCall` RPC timeout, a temporary `getWithdrawCapIDCached` miss, or a `getWithdrawAndCallObjectRefs` network failure — will now permanently cancel the withdrawal instead of bubbling the error back to the scheduler for a retry. The existing `ErrInvalidPayload` / `ErrObjectOwnership` branches already cover the known deterministic cases; the remaining errors are a mix of deterministic (e.g. invalid gas price, unsupported coin type) and genuinely transient (RPC connectivity). Consider distinguishing the two categories — or at minimum wrapping the specific Sui-type-argument error as a new sentinel — so that transient failures can still surface as retryable errors.

How can I resolve this? If you propose a fix, please make it concise.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Addressed on the current branch: the generic err != nil branch now returns errors.Wrap(err, "unable to build withdraw tx") instead of broadcasting the cancel transaction. Only the deterministic ErrInvalidPayload and ErrObjectOwnership cases still trigger broadcastCancelTx, so transient build/RPC errors remain retryable.

}

req := models.SuiExecuteTransactionBlockRequest{
Expand Down
Loading