aarifullin/neoneo-go
1
0
Fork 0
forked from TrueCloudLab/neoneo-go
Code Pull requests Activity

core: do not allow NEP17 roundtrip in case of insufficient funds

Browse source 
NEP17 roundtrip is prohibited if from account doesn't have enough funds.

This commit fixes states diff in block 92057 where account
NfuwpaQ1A2xaeVbxWe8FRtaRgaMa8yF3YM initiates two NEO roundtrips with
amount exceeding the account's balance:

block 92057: value mismatch for key +////xTbYWBH3r5qhRKZAPFPHabKfb2vhQ==: QQMhAkwBIQOZZwEA vs QQMhAkwBIQN/ZwEA
block 92057: value mismatch for key +v///ws=: kqlddcitCg== vs tphddcitCg==
block 92057: value mismatch for key +v///xTbYWBH3r5qhRKZAPFPHabKfb2vhQ==: QQEhBUWyDu0W vs QQEhBWmhDu0W

C#'s applog (contains False and False on stack for both transfers):
```
{
   "id" : 1,
   "jsonrpc" : "2.0",
   "result" : {
      "executions" : [
         {
            "gasconsumed" : "11955500",
            "exception" : null,
            "stack" : [
               {
                  "value" : false,
                  "type" : "Boolean"
               },
               {
                  "value" : false,
                  "type" : "Boolean"
               }
            ],
            "vmstate" : "HALT",
            "trigger" : "Application",
            "notifications" : []
         }
      ],
      "txid" : "0x8e73a7e9a566a514813907272ad65fc965002c3b098eacc5bdda529af19d7688"
   }
}
```

Go's applog (both transfers succeeded and GAS minted):
```
{
   "result" : {
      "executions" : [
         {
            "gasconsumed" : "11955500",
            "trigger" : "Application",
            "stack" : [
               {
                  "type" : "Boolean",
                  "value" : true
               },
               {
                  "type" : "Boolean",
                  "value" : true
               }
            ],
            "vmstate" : "HALT",
            "notifications" : [
               {
                  "eventname" : "Transfer",
                  "contract" : "0xd2a4cff31913016155e38e474a2c06d08be276cf",
                  "state" : {
                     "value" : [
                        {
                           "type" : "Any"
                        },
                        {
                           "value" : "22FgR96+aoUSmQDxTx2myn29r4U=",
                           "type" : "ByteString"
                        },
                        {
                           "value" : "4316",
                           "type" : "Integer"
                        }
                     ],
                     "type" : "Array"
                  }
               },
               {
                  "state" : {
                     "type" : "Array",
                     "value" : [
                        {
                           "value" : "22FgR96+aoUSmQDxTx2myn29r4U=",
                           "type" : "ByteString"
                        },
                        {
                           "type" : "ByteString",
                           "value" : "22FgR96+aoUSmQDxTx2myn29r4U="
                        },
                        {
                           "type" : "Integer",
                           "value" : "1111111111"
                        }
                     ]
                  },
                  "contract" : "0xef4073a0f2b305a38ec4050e4d3d28bc40ea63f5",
                  "eventname" : "Transfer"
               },
               {
                  "contract" : "0xef4073a0f2b305a38ec4050e4d3d28bc40ea63f5",
                  "state" : {
                     "type" : "Array",
                     "value" : [
                        {
                           "value" : "22FgR96+aoUSmQDxTx2myn29r4U=",
                           "type" : "ByteString"
                        },
                        {
                           "type" : "ByteString",
                           "value" : "22FgR96+aoUSmQDxTx2myn29r4U="
                        },
                        {
                           "type" : "Integer",
                           "value" : "1111111"
                        }
                     ]
                  },
                  "eventname" : "Transfer"
               }
            ]
         }
      ],
      "txid" : "0x8e73a7e9a566a514813907272ad65fc965002c3b098eacc5bdda529af19d7688"
   },
   "id" : 1,
   "jsonrpc" : "2.0"
}
```
This commit is contained in:
Anna Shaleva 2021-06-09 11:58:58 +03:00
parent 0a118cb476
commit be902afe9e
4 changed files with 115 additions and 5 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

14
pkg/core/helper_test.go
View file

@ -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)

26
pkg/core/native/native_nep17.go
View file

@ -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
}
}

48
pkg/core/native_gas_test.go
View file

@ -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)
})
}

32
pkg/core/native_neo_test.go
View file

@ -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)
})
}