From e0e7fdf8108eea8c10acda5aab7c9a23726b08b8 Mon Sep 17 00:00:00 2001 From: Anna Shaleva Date: Mon, 1 Apr 2024 20:51:33 +0300 Subject: [PATCH] cli: unify `canceltx` command output Always return "Target transaction accepted" error if the target transation was either accepted by the moment of command run or during the command handling. Exit with code 1 in both cases. Adjust TestAwaitUtilCancelTx correspondingly, allow both cases in test. Simplify the test along the way, remove useless code. Close #3365. Signed-off-by: Anna Shaleva --- cli/util/cancel.go | 19 +++++++++---------- cli/util/util_test.go | 25 +++++++------------------ internal/testcli/executor.go | 14 ++++++++++++++ 3 files changed, 30 insertions(+), 28 deletions(-) diff --git a/cli/util/cancel.go b/cli/util/cancel.go index 531b941b6..fbb112ad8 100644 --- a/cli/util/cancel.go +++ b/cli/util/cancel.go @@ -52,7 +52,7 @@ func cancelTx(ctx *cli.Context) error { mainTx, _ := c.GetRawTransactionVerbose(txHash) if mainTx != nil && !mainTx.Blockhash.Equals(util.Uint256{}) { - return cli.NewExitError(fmt.Errorf("transaction %s is already accepted at block %s", txHash, mainTx.Blockhash.StringLE()), 1) + return cli.NewExitError(fmt.Errorf("target transaction %s is accepted at block %s", txHash, mainTx.Blockhash.StringLE()), 1) } if mainTx != nil && !mainTx.HasSigner(acc.ScriptHash()) { @@ -76,10 +76,7 @@ func cancelTx(ctx *cli.Context) error { if err != nil { return cli.NewExitError(fmt.Errorf("failed to send conflicting transaction: %w", err), 1) } - var ( - acceptedH = resHash - res *state.AppExecResult - ) + var res *state.AppExecResult if ctx.Bool("await") { res, err = a.WaitAny(gctx, resVub, txHash, resHash) if err != nil { @@ -93,12 +90,14 @@ func cancelTx(ctx *cli.Context) error { return cli.NewExitError(fmt.Errorf("failed to await target/ conflicting transaction %s/ %s: %w", txHash.StringLE(), resHash.StringLE(), err), 1) } if txHash.Equals(res.Container) { - fmt.Fprintln(ctx.App.Writer, "Target transaction accepted") - acceptedH = txHash - } else { - fmt.Fprintln(ctx.App.Writer, "Conflicting transaction accepted") + tx, err := c.GetRawTransactionVerbose(txHash) + if err != nil { + return cli.NewExitError(fmt.Errorf("target transaction %s is accepted", txHash), 1) + } + return cli.NewExitError(fmt.Errorf("target transaction %s is accepted at block %s", txHash, tx.Blockhash.StringLE()), 1) } + fmt.Fprintln(ctx.App.Writer, "Conflicting transaction accepted") } - txctx.DumpTransactionInfo(ctx.App.Writer, acceptedH, res) + txctx.DumpTransactionInfo(ctx.App.Writer, resHash, res) return nil } diff --git a/cli/util/util_test.go b/cli/util/util_test.go index 4787576fa..3a32302cd 100644 --- a/cli/util/util_test.go +++ b/cli/util/util_test.go @@ -1,9 +1,9 @@ package util_test import ( + "fmt" "os" "path/filepath" - "strings" "testing" "time" @@ -168,24 +168,13 @@ func TestAwaitUtilCancelTx(t *testing.T) { _, ok := e.Chain.GetMemPool().TryGetValue(txHash) require.True(t, ok) + // Allow both cases: either target or conflicting tx acceptance. e.In.WriteString("one\r") - e.Run(t, append(args, txHash.StringLE())...) - - response := e.GetNextLine(t) - if strings.Contains(response, "Conflicting transaction accepted") { + err = e.RunOrError(t, fmt.Sprintf("target transaction %s is accepted", txHash), append(args, txHash.StringLE())...) + if err == nil { + response := e.GetNextLine(t) + require.Equal(t, "Conflicting transaction accepted", response) resHash, _ := e.CheckAwaitableTxPersisted(t) - require.Eventually(t, func() bool { - _, aerErr := e.Chain.GetAppExecResults(resHash.Hash(), trigger.Application) - return aerErr == nil - }, time.Second*2, time.Millisecond*50) - } else if strings.Contains(response, "Target transaction accepted") { - require.Eventually(t, func() bool { - _, _, err := e.Chain.GetTransaction(txHash) - require.NoError(t, err, "original transaction should be on chain") - _, aerErr := e.Chain.GetAppExecResults(txHash, trigger.Application) - return aerErr == nil - }, time.Second*2, time.Millisecond*50) - } else { - t.Fatalf("unexpected response: %s", response) + require.NotEqual(t, resHash, txHash) } } diff --git a/internal/testcli/executor.go b/internal/testcli/executor.go index 877b250e0..d29a8d026 100644 --- a/internal/testcli/executor.go +++ b/internal/testcli/executor.go @@ -286,6 +286,20 @@ func (e *Executor) Run(t *testing.T, args ...string) { require.NoError(t, e.run(args...)) checkExit(t, ch, 0) } + +// RunOrError runs command and checks that if there was an error, then its text matches the provided one. +func (e *Executor) RunOrError(t *testing.T, errText string, args ...string) error { + ch := setExitFunc() + err := e.run(args...) + if err != nil { + require.True(t, strings.Contains(err.Error(), errText)) + checkExit(t, ch, 1) + } else { + checkExit(t, ch, 0) + } + return err +} + func (e *Executor) run(args ...string) error { e.Out.Reset() e.Err.Reset()