Skip to content

Commit

Permalink
balance: add prefix to storage, fix #307
Browse files Browse the repository at this point in the history
Prefix-less storage is dangerous, can be easily mixed with something else
for different calls.

Signed-off-by: Roman Khimov <roman@nspcc.ru>
  • Loading branch information
roman-khimov committed Jun 20, 2024
1 parent 2d029d5 commit 4797148
Show file tree
Hide file tree
Showing 6 changed files with 130 additions and 10 deletions.
43 changes: 36 additions & 7 deletions contracts/balance/contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const (
symbol = "NEOFS"
decimals = 12
circulation = "MainnetGAS"
accPrefix = 'a'
)

var token Token
Expand Down Expand Up @@ -70,6 +71,9 @@ func _deploy(data any, isUpdate bool) {
if version < 17_000 {
switchToNotary(ctx)
}
if version < 20_000 {
switchToAccPrefixes(ctx)
}

return
}
Expand Down Expand Up @@ -110,6 +114,27 @@ func switchToNotary(ctx storage.Context) {
}
}

// switchToAccPrefixes moves account data to a specific prefix to avoid any
// collisions with other things stored in the contract.
//
// nolint:unused
func switchToAccPrefixes(ctx storage.Context) {
it := storage.Find(ctx, []byte{}, storage.None)
for iterator.Next(it) {
item := iterator.Value(it).(struct {
key []byte
value []byte
})

if len(item.key) != interop.Hash160Len {
continue
}

storage.Put(ctx, append([]byte{accPrefix}, item.key...), item.value)
storage.Delete(ctx, item.key)
}
}

// Update method updates contract source code and manifest. It can be invoked
// only by committee.
func Update(script []byte, manifest []byte, data any) {
Expand Down Expand Up @@ -201,7 +226,7 @@ func Lock(txDetails []byte, from, to interop.Hash160, amount, until int) {
Parent: from,
}

common.SetSerialized(ctx, to, lockAccount)
common.SetSerialized(ctx, append([]byte{accPrefix}, to...), lockAccount)

result := token.transfer(ctx, from, to, amount, true, details)
if !result {
Expand All @@ -224,7 +249,7 @@ func NewEpoch(epochNum int) {
multiaddr := common.AlphabetAddress()
common.CheckAlphabetWitness(multiaddr)

it := storage.Find(ctx, []byte{}, storage.KeysOnly)
it := storage.Find(ctx, []byte{accPrefix}, storage.KeysOnly|storage.RemovePrefix)
for iterator.Next(it) {
addr := iterator.Value(it).(interop.Hash160) // it MUST BE `storage.KeysOnly`
if len(addr) != interop.Hash160Len {
Expand Down Expand Up @@ -335,18 +360,22 @@ func (t Token) transfer(ctx storage.Context, from, to interop.Hash160, amount in
}

if len(from) == interop.Hash160Len {
var fromKey = append([]byte{accPrefix}, from...)

if amountFrom.Balance == amount {
storage.Delete(ctx, from)
storage.Delete(ctx, fromKey)
} else {
amountFrom.Balance -= amount
common.SetSerialized(ctx, from, amountFrom)
common.SetSerialized(ctx, fromKey, amountFrom)
}
}

if len(to) == interop.Hash160Len {
var toKey = append([]byte{accPrefix}, to...)

amountTo := getAccount(ctx, to)
amountTo.Balance += amount
common.SetSerialized(ctx, to, amountTo)
common.SetSerialized(ctx, toKey, amountTo)
}

runtime.Notify("Transfer", from, to, amount)
Expand Down Expand Up @@ -397,8 +426,8 @@ func isUsableAddress(addr interop.Hash160) bool {
return false
}

func getAccount(ctx storage.Context, key any) Account {
data := storage.Get(ctx, key)
func getAccount(ctx storage.Context, key interop.Hash160) Account {
data := storage.Get(ctx, append([]byte{accPrefix}, key...))
if data != nil {
return std.Deserialize(data.([]byte)).(Account)
}
Expand Down
Binary file modified contracts/balance/contract.nef
Binary file not shown.
2 changes: 1 addition & 1 deletion contracts/balance/doc.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ Contract storage model.
Key-value storage format:
- 'MainnetGAS' -> int
total amount of Mainchain GAS deployed in the NeoFS network in Fixed12
- interop.Hash160 -> std.Serialize(Account)
- a<interop.Hash160> -> std.Serialize(Account)
balance sheet of all NeoFS users (here Account is a structure defined in current package)
# Accounting
Expand Down
2 changes: 1 addition & 1 deletion contracts/balance/manifest.json
Original file line number Diff line number Diff line change
@@ -1 +1 @@
{"name":"NeoFS Balance","abi":{"methods":[{"name":"_initialize","offset":0,"parameters":[],"returntype":"Void","safe":false},{"name":"_deploy","offset":93,"parameters":[{"name":"data","type":"Any"},{"name":"isUpdate","type":"Boolean"}],"returntype":"Void","safe":false},{"name":"balanceOf","offset":1432,"parameters":[{"name":"account","type":"Hash160"}],"returntype":"Integer","safe":true},{"name":"burn","offset":1965,"parameters":[{"name":"from","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"txDetails","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"decimals","offset":1410,"parameters":[],"returntype":"Integer","safe":true},{"name":"lock","offset":1578,"parameters":[{"name":"txDetails","type":"ByteArray"},{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"until","type":"Integer"}],"returntype":"Void","safe":false},{"name":"mint","offset":1844,"parameters":[{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"txDetails","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"newEpoch","offset":1715,"parameters":[{"name":"epochNum","type":"Integer"}],"returntype":"Void","safe":false},{"name":"symbol","offset":1406,"parameters":[],"returntype":"String","safe":true},{"name":"totalSupply","offset":1414,"parameters":[],"returntype":"Integer","safe":true},{"name":"transfer","offset":1451,"parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"data","type":"Any"}],"returntype":"Boolean","safe":false},{"name":"transferX","offset":1475,"parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"details","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"update","offset":1275,"parameters":[{"name":"script","type":"ByteArray"},{"name":"manifest","type":"ByteArray"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false},{"name":"version","offset":2116,"parameters":[],"returntype":"Integer","safe":true}],"events":[{"name":"Lock","parameters":[{"name":"txID","type":"ByteArray"},{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"until","type":"Integer"}]},{"name":"Transfer","parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"}]},{"name":"TransferX","parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"details","type":"ByteArray"}]}]},"features":{},"groups":[],"permissions":[{"contract":"*","methods":["update","subscribeForNewEpoch"]}],"supportedstandards":["NEP-17"],"trusts":[],"extra":null}
{"name":"NeoFS Balance","abi":{"methods":[{"name":"_initialize","offset":0,"parameters":[],"returntype":"Void","safe":false},{"name":"_deploy","offset":93,"parameters":[{"name":"data","type":"Any"},{"name":"isUpdate","type":"Boolean"}],"returntype":"Void","safe":false},{"name":"balanceOf","offset":1549,"parameters":[{"name":"account","type":"Hash160"}],"returntype":"Integer","safe":true},{"name":"burn","offset":2101,"parameters":[{"name":"from","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"txDetails","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"decimals","offset":1527,"parameters":[],"returntype":"Integer","safe":true},{"name":"lock","offset":1695,"parameters":[{"name":"txDetails","type":"ByteArray"},{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"until","type":"Integer"}],"returntype":"Void","safe":false},{"name":"mint","offset":1980,"parameters":[{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"txDetails","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"newEpoch","offset":1850,"parameters":[{"name":"epochNum","type":"Integer"}],"returntype":"Void","safe":false},{"name":"symbol","offset":1523,"parameters":[],"returntype":"String","safe":true},{"name":"totalSupply","offset":1531,"parameters":[],"returntype":"Integer","safe":true},{"name":"transfer","offset":1568,"parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"data","type":"Any"}],"returntype":"Boolean","safe":false},{"name":"transferX","offset":1592,"parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"details","type":"ByteArray"}],"returntype":"Void","safe":false},{"name":"update","offset":1392,"parameters":[{"name":"script","type":"ByteArray"},{"name":"manifest","type":"ByteArray"},{"name":"data","type":"Any"}],"returntype":"Void","safe":false},{"name":"version","offset":2252,"parameters":[],"returntype":"Integer","safe":true}],"events":[{"name":"Lock","parameters":[{"name":"txID","type":"ByteArray"},{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"until","type":"Integer"}]},{"name":"Transfer","parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"}]},{"name":"TransferX","parameters":[{"name":"from","type":"Hash160"},{"name":"to","type":"Hash160"},{"name":"amount","type":"Integer"},{"name":"details","type":"ByteArray"}]}]},"features":{},"groups":[],"permissions":[{"contract":"*","methods":["update","subscribeForNewEpoch"]}],"supportedstandards":["NEP-17"],"trusts":[],"extra":null}
28 changes: 27 additions & 1 deletion contracts/balance/migration_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
package balance_test

import (
"math/big"
"testing"

"github.com/nspcc-dev/neo-go/pkg/util"
"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
"github.com/nspcc-dev/neofs-contract/tests/dump"
"github.com/nspcc-dev/neofs-contract/tests/migration"
Expand Down Expand Up @@ -60,7 +62,25 @@ func testMigrationFromDump(t *testing.T, d *dump.Reader) {
// try to update the contract
if notaryDisabled && prevPendingVotes {
c.CheckUpdateFail(t, "pending vote detected")
return
t.Fail()
}

var accounts = []util.Uint160{}

c.SeekStorage([]byte{}, func(k, v []byte) bool {
if len(k) == util.Uint160Size {
a, err := util.Uint160DecodeBytesBE(k)
require.NoError(t, err)
accounts = append(accounts, a)
}
return true
})

var balances = []*big.Int{}
for i := range accounts {
n, err := c.Call(t, "balanceOf", accounts[i]).TryInteger()
require.NoError(t, err)
balances = append(balances, n)
}

c.CheckUpdateSuccess(t)
Expand All @@ -75,4 +95,10 @@ func testMigrationFromDump(t *testing.T, d *dump.Reader) {
require.Nil(t, c.GetStorageItem([]byte("netmapScriptHash")), "Netmap contract address should be removed")

require.Equal(t, prevTotalSupply, newTotalSupply)

for i := range accounts {
n, err := c.Call(t, "balanceOf", accounts[i]).TryInteger()
require.NoError(t, err)
require.Equal(t, balances[i], n)
}
}
65 changes: 65 additions & 0 deletions tests/balance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"github.com/nspcc-dev/neo-go/pkg/neotest"
"github.com/nspcc-dev/neo-go/pkg/util"
"github.com/nspcc-dev/neo-go/pkg/vm/stackitem"
"github.com/stretchr/testify/require"
)

const balancePath = "../contracts/balance"
Expand All @@ -27,3 +28,67 @@ func deployBalanceContract(t *testing.T, e *neotest.Executor, addrNetmap, addrCo
func balanceMint(t *testing.T, c *neotest.ContractInvoker, acc neotest.Signer, amount int64, details []byte) {
c.Invoke(t, stackitem.Null{}, "mint", acc.ScriptHash(), amount, details)
}

func TestBalanceLifecycle(t *testing.T) {
e := newExecutor(t)

deployDefaultNNS(t, e)
nHash := deployNetmapContract(t, e)
bHash := deployBalanceContract(t, e, util.Uint160{}, util.Uint160{})

c := e.CommitteeInvoker(bHash)
acc1 := c.NewAccount(t)
acc2 := c.NewAccount(t)

checkTotalSupply := func(expected int64) {
stack, err := c.TestInvoke(t, "totalSupply")
require.NoError(t, err)
require.Equal(t, 1, stack.Len())
bal := stack.Pop().BigInt().Int64()
require.Equal(t, expected, bal)
}

checkBalance := func(acc util.Uint160, expected int64) {
stack, err := c.TestInvoke(t, "balanceOf", acc)
require.NoError(t, err)
require.Equal(t, 1, stack.Len())
bal := stack.Pop().BigInt().Int64()
require.Equal(t, expected, bal)
}

c.Invoke(t, stackitem.Null{}, "mint", acc1.ScriptHash(), 100500, []byte("extra"))
checkBalance(acc1.ScriptHash(), 100500)
checkTotalSupply(100500)

// Fails because acc1 is not a signer.
c.Invoke(t, false, "transfer", acc1.ScriptHash(), acc2.ScriptHash(), 500, nil)

c1 := c.WithSigners(acc1)
c1.Invoke(t, true, "transfer", acc1.ScriptHash(), acc2.ScriptHash(), 500, nil)
checkBalance(acc2.ScriptHash(), 500)
checkBalance(acc1.ScriptHash(), 100000)
checkTotalSupply(100500)

// But c1 can't mint.
c1.InvokeFail(t, "alphabet witness check failed", "mint", acc1.ScriptHash(), 100500, []byte("extra"))

// c1 can't burn.
c1.InvokeFail(t, "alphabet witness check failed", "burn", acc1.ScriptHash(), 1, []byte("extra"))

// Validators can burn.
c.Invoke(t, stackitem.Null{}, "burn", acc1.ScriptHash(), 1, []byte("extra"))
checkBalance(acc1.ScriptHash(), 99999)
checkTotalSupply(100499)

c.Invoke(t, stackitem.Null{}, "lock", []byte("details"), acc1.ScriptHash(), util.Uint160{1, 2, 3}, 999, 1)
checkBalance(util.Uint160{1, 2, 3}, 999)
checkBalance(acc1.ScriptHash(), 99000)
checkTotalSupply(100499)

nc := e.CommitteeInvoker(nHash)
nc.Invoke(t, stackitem.Null{}, "newEpoch", 1)
// Unlocked.
checkBalance(acc1.ScriptHash(), 99999)
checkBalance(util.Uint160{1, 2, 3}, 0)
checkTotalSupply(100499)
}

0 comments on commit 4797148

Please sign in to comment.