diff --git a/pkg/core/helper_test.go b/pkg/core/helper_test.go index 14bb9bc3e..227dce033 100644 --- a/pkg/core/helper_test.go +++ b/pkg/core/helper_test.go @@ -498,9 +498,15 @@ func initBasicChain(t *testing.T, bc *Blockchain) { } func newNEP17Transfer(sc, from, to util.Uint160, amount int64, additionalArgs ...interface{}) *transaction.Transaction { + return newNEP17TransferWithAssert(sc, from, to, amount, true, additionalArgs...) +} + +func newNEP17TransferWithAssert(sc, from, to util.Uint160, amount int64, needAssert bool, additionalArgs ...interface{}) *transaction.Transaction { w := io.NewBufBinWriter() emit.AppCall(w.BinWriter, sc, "transfer", callflag.All, from, to, amount, additionalArgs) - emit.Opcodes(w.BinWriter, opcode.ASSERT) + if needAssert { + emit.Opcodes(w.BinWriter, opcode.ASSERT) + } if w.Err != nil { panic(fmt.Errorf("failed to create nep17 transfer transaction: %w", w.Err)) } @@ -677,7 +683,11 @@ func transferTokenFromMultisigAccountCheckOK(t *testing.T, chain *Blockchain, to } func transferTokenFromMultisigAccount(t *testing.T, chain *Blockchain, to, tokenHash util.Uint160, amount int64, additionalArgs ...interface{}) *transaction.Transaction { - transferTx := newNEP17Transfer(tokenHash, testchain.MultisigScriptHash(), to, amount, additionalArgs...) + return transferTokenFromMultisigAccountWithAssert(t, chain, to, tokenHash, amount, true, additionalArgs...) +} + +func transferTokenFromMultisigAccountWithAssert(t *testing.T, chain *Blockchain, to, tokenHash util.Uint160, amount int64, needAssert bool, additionalArgs ...interface{}) *transaction.Transaction { + transferTx := newNEP17TransferWithAssert(tokenHash, testchain.MultisigScriptHash(), to, amount, needAssert, additionalArgs...) transferTx.SystemFee = 100000000 transferTx.ValidUntilBlock = chain.BlockHeight() + 1 addSigners(neoOwner, transferTx) diff --git a/pkg/core/native/native_nep17.go b/pkg/core/native/native_nep17.go index bbc55bec7..4e2b9deb6 100644 --- a/pkg/core/native/native_nep17.go +++ b/pkg/core/native/native_nep17.go @@ -163,7 +163,9 @@ func (c *nep17TokenNative) emitTransfer(ic *interop.Context, from, to *util.Uint ic.Notifications = append(ic.Notifications, ne) } -func (c *nep17TokenNative) updateAccBalance(ic *interop.Context, acc util.Uint160, amount *big.Int) error { +// updateAccBalance adds specified amount to the acc's balance. If requiredBalance +// is set and amount is 0, then acc's balance is checked against requiredBalance. +func (c *nep17TokenNative) updateAccBalance(ic *interop.Context, acc util.Uint160, amount *big.Int, requiredBalance *big.Int) error { key := makeAccountKey(acc) si := ic.DAO.GetStorageItem(c.ID, key) if si == nil { @@ -171,6 +173,24 @@ func (c *nep17TokenNative) updateAccBalance(ic *interop.Context, acc util.Uint16 return errors.New("insufficient funds") } si = state.StorageItem{} + } else if amount.Sign() == 0 && requiredBalance != nil { + // If amount == 0 then it's either a roundtrip or an empty transfer. In + // case of a roundtrip account's balance may still be less than actual + // transfer's amount, so we need to check it. Other cases are handled by + // `incBalance` method. + balance, err := c.balFromBytes(&si) + if err != nil { + return fmt.Errorf("failed to deserialise balance: %w", err) + } + if balance.Cmp(requiredBalance) < 0 { + // Firstly, need to put it back to storage as it affects dumps. + err = ic.DAO.PutStorageItem(c.ID, key, si) + if err != nil { + return err + } + // Finally, return an error. + return errors.New("insufficient funds") + } } err := c.incBalance(ic, acc, &si, amount) @@ -207,12 +227,12 @@ func (c *nep17TokenNative) TransferInternal(ic *interop.Context, from, to util.U } else { inc = new(big.Int).Neg(inc) } - if err := c.updateAccBalance(ic, from, inc); err != nil { + if err := c.updateAccBalance(ic, from, inc, amount); err != nil { return err } if !isEmpty { - if err := c.updateAccBalance(ic, to, amount); err != nil { + if err := c.updateAccBalance(ic, to, amount, nil); err != nil { return err } } diff --git a/pkg/core/native_gas_test.go b/pkg/core/native_gas_test.go index 8b7e06e9d..dbc358f95 100644 --- a/pkg/core/native_gas_test.go +++ b/pkg/core/native_gas_test.go @@ -5,7 +5,10 @@ import ( "testing" "github.com/nspcc-dev/neo-go/internal/random" + "github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger" + "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" + "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" "github.com/nspcc-dev/neo-go/pkg/wallet" "github.com/stretchr/testify/require" ) @@ -68,3 +71,48 @@ func TestGAS_Refuel(t *testing.T) { require.Equal(t, vm.FaultState, aer.VMState) }) } + +func TestGAS_Roundtrip(t *testing.T) { + bc := newTestChain(t) + + getUtilityTokenBalance := func(bc *Blockchain, acc util.Uint160) (*big.Int, uint32) { + bs, err := bc.dao.GetNEP17Balances(acc) + if err != nil { + return big.NewInt(0), 0 + } + balance := bs.Trackers[bc.contracts.GAS.ID] + return &balance.Balance, balance.LastUpdatedBlock + } + + initialBalance, _ := getUtilityTokenBalance(bc, neoOwner) + require.NotNil(t, initialBalance) + + t.Run("bad: amount > initial balance", func(t *testing.T) { + tx := transferTokenFromMultisigAccountWithAssert(t, bc, neoOwner, bc.contracts.GAS.Hash, initialBalance.Int64()+1, false) + aer, err := bc.GetAppExecResults(tx.Hash(), trigger.Application) + require.NoError(t, err) + require.Equal(t, vm.HaltState, aer[0].VMState, aer[0].FaultException) // transfer without assert => HALT state + checkResult(t, &aer[0], stackitem.NewBool(false)) + require.Len(t, aer[0].Events, 0) // failed transfer => no events + // check balance and height were not changed + updatedBalance, updatedHeight := getUtilityTokenBalance(bc, neoOwner) + initialBalance.Sub(initialBalance, big.NewInt(tx.SystemFee+tx.NetworkFee)) + require.Equal(t, initialBalance, updatedBalance) + require.Equal(t, bc.BlockHeight(), updatedHeight) + }) + + t.Run("good: amount < initial balance", func(t *testing.T) { + amount := initialBalance.Int64() - 10_00000000 + tx := transferTokenFromMultisigAccountWithAssert(t, bc, neoOwner, bc.contracts.GAS.Hash, amount, false) + aer, err := bc.GetAppExecResults(tx.Hash(), trigger.Application) + require.NoError(t, err) + require.Equal(t, vm.HaltState, aer[0].VMState, aer[0].FaultException) + checkResult(t, &aer[0], stackitem.NewBool(true)) + require.Len(t, aer[0].Events, 1) // roundtrip + // check balance wasn't changed and height was updated + updatedBalance, updatedHeight := getUtilityTokenBalance(bc, neoOwner) + initialBalance.Sub(initialBalance, big.NewInt(tx.SystemFee+tx.NetworkFee)) + require.Equal(t, initialBalance, updatedBalance) + require.Equal(t, bc.BlockHeight(), updatedHeight) + }) +} diff --git a/pkg/core/native_neo_test.go b/pkg/core/native_neo_test.go index f5766d565..87ff08ab4 100644 --- a/pkg/core/native_neo_test.go +++ b/pkg/core/native_neo_test.go @@ -325,3 +325,35 @@ func TestRegisterPrice(t *testing.T) { testGetSet(t, bc, bc.contracts.NEO.Hash, "RegisterPrice", native.DefaultRegisterPrice, 1, math.MaxInt64) } + +func TestNEO_Roundtrip(t *testing.T) { + bc := newTestChain(t) + initialBalance, initialHeight := bc.GetGoverningTokenBalance(neoOwner) + require.NotNil(t, initialBalance) + + t.Run("bad: amount > initial balance", func(t *testing.T) { + tx := transferTokenFromMultisigAccountWithAssert(t, bc, neoOwner, bc.contracts.NEO.Hash, initialBalance.Int64()+1, false) + aer, err := bc.GetAppExecResults(tx.Hash(), trigger.Application) + require.NoError(t, err) + require.Equal(t, vm.HaltState, aer[0].VMState, aer[0].FaultException) // transfer without assert => HALT state + checkResult(t, &aer[0], stackitem.NewBool(false)) + require.Len(t, aer[0].Events, 0) // failed transfer => no events + // check balance and height were not changed + updatedBalance, updatedHeight := bc.GetGoverningTokenBalance(neoOwner) + require.Equal(t, initialBalance, updatedBalance) + require.Equal(t, initialHeight, updatedHeight) + }) + + t.Run("good: amount == initial balance", func(t *testing.T) { + tx := transferTokenFromMultisigAccountWithAssert(t, bc, neoOwner, bc.contracts.NEO.Hash, initialBalance.Int64(), false) + aer, err := bc.GetAppExecResults(tx.Hash(), trigger.Application) + require.NoError(t, err) + require.Equal(t, vm.HaltState, aer[0].VMState, aer[0].FaultException) + checkResult(t, &aer[0], stackitem.NewBool(true)) + require.Len(t, aer[0].Events, 2) // roundtrip + GAS claim + // check balance wasn't changed and height was updated + updatedBalance, updatedHeight := bc.GetGoverningTokenBalance(neoOwner) + require.Equal(t, initialBalance, updatedBalance) + require.Equal(t, bc.BlockHeight(), updatedHeight) + }) +}