[#45] nns: Fix inconsistent fee of register operations

Signed-off-by: Alex Vanin <a.vanin@yadro.com>

common/storage.go

@@ -1,6 +1,7 @@
 package common
 
 import (
+	"github.com/nspcc-dev/neo-go/pkg/interop/convert"
 	"github.com/nspcc-dev/neo-go/pkg/interop/native/std"
 	"github.com/nspcc-dev/neo-go/pkg/interop/storage"
 )
@@ -10,3 +11,18 @@ func SetSerialized(ctx storage.Context, key interface{}, value interface{}) {
 	data := std.Serialize(value)
 	storage.Put(ctx, key, data)
 }
+
+// ToFixedWidth64 converts x to bytes such that numbers <= math.MaxUint64
+// have constant with of 9.
+func ToFixedWidth64(x int) []byte {
+	data := convert.ToBytes(x)
+	if x < 0 || len(data) >= 9 {
+		return data
+	}
+	return append(data, make([]byte, 9-len(data))...)
+}
+
+// FromFixedWidth64 is a reverse function for ToFixedWidth64.
+func FromFixedWidth64(x []byte) int {
+	return convert.ToInteger(x)
+}

container/container_contract.go

@@ -343,6 +343,11 @@ type DelInfo struct {
 	Epoch int
 }
 
+type delInfo struct {
+	Owner []byte
+	Epoch []byte
+}
+
 // DeletionInfo method returns container deletion info.
 // If the container had never existed, NotFoundError is throwed.
 // It can be used to check whether non-existing container was indeed deleted
@@ -354,7 +359,12 @@ func DeletionInfo(containerID []byte) DelInfo {
 	if data == nil {
 		panic(NotFoundError)
 	}
-	return std.Deserialize(data).(DelInfo)
+
+	d := std.Deserialize(data).(delInfo)
+	return DelInfo{
+		Owner: d.Owner,
+		Epoch: common.FromFixedWidth64(d.Epoch),
+	}
 }
 
 // Get method returns a structure that contains a stable marshaled Container structure,
@@ -631,9 +641,9 @@ func removeContainer(ctx storage.Context, id []byte, owner []byte) {
 	graveKey := append([]byte{graveKeyPrefix}, id...)
 	netmapContractAddr := storage.Get(ctx, netmapContractKey).(interop.Hash160)
 	epoch := contract.Call(netmapContractAddr, "epoch", contract.ReadOnly).(int)
-	common.SetSerialized(ctx, graveKey, DelInfo{
+	common.SetSerialized(ctx, graveKey, delInfo{
 		Owner: owner,
-		Epoch: epoch,
+		Epoch: common.ToFixedWidth64(epoch),
 	})
 }
 

nns/nns_contract.go

@@ -458,13 +458,13 @@ func updateBalance(ctx storage.Context, tokenId []byte, acc interop.Hash160, dif
 	balanceKey := append([]byte{prefixBalance}, acc...)
 	var balance int
 	if b := storage.Get(ctx, balanceKey); b != nil {
-		balance = b.(int)
+		balance = common.FromFixedWidth64(b.([]byte))
 	}
 	balance += diff
 	if balance == 0 {
 		storage.Delete(ctx, balanceKey)
 	} else {
-		storage.Put(ctx, balanceKey, balance)
+		storage.Put(ctx, balanceKey, common.ToFixedWidth64(balance))
 	}
 
 	tokenKey := getTokenKey(tokenId)
@@ -488,14 +488,14 @@ func postTransfer(from, to interop.Hash160, tokenID []byte, data interface{}) {
 // getTotalSupply returns total supply from storage.
 func getTotalSupply(ctx storage.Context) int {
 	val := storage.Get(ctx, []byte{prefixTotalSupply})
-	return val.(int)
+	return common.FromFixedWidth64(val.([]byte))
 }
 
 // updateTotalSupply adds the specified diff to the total supply.
 func updateTotalSupply(ctx storage.Context, diff int) {
 	tsKey := []byte{prefixTotalSupply}
 	ts := getTotalSupply(ctx)
-	storage.Put(ctx, tsKey, ts+diff)
+	storage.Put(ctx, tsKey, common.ToFixedWidth64(ts+diff))
 }
 
 // getTokenKey computes hash160 from the given tokenID.

tests/common_test.go

@@ -0,0 +1,61 @@
+package tests
+
+import (
+	"math"
+	"math/big"
+	"path"
+	"testing"
+
+	"github.com/nspcc-dev/neo-go/pkg/neotest"
+	"github.com/stretchr/testify/require"
+)
+
+const testdataPath = "./testdata"
+
+func newTestdataInvoker(t *testing.T) *neotest.ContractInvoker {
+	e := newExecutor(t)
+	ctr := neotest.CompileFile(t, e.CommitteeHash, testdataPath, path.Join(testdataPath, "config.yml"))
+	e.DeployContract(t, ctr, nil)
+	return e.CommitteeInvoker(ctr.Hash)
+}
+
+func TestEncodeU64(t *testing.T) {
+	// Let's check boundary values for all bit sizes:
+	var nums []uint64
+	for i := 0; i < 64; i++ {
+		if i != 0 {
+			nums = append(nums, (1<<i)-1)
+		}
+		nums = append(nums, 1<<i)
+		if i != 63 {
+			nums = append(nums, (1<<i)+1)
+		}
+	}
+
+	c := newTestdataInvoker(t)
+	for _, n := range nums {
+		v, err := c.TestInvoke(t, "encode", n)
+		require.NoError(t, err)
+		require.Equal(t, 1, v.Len())
+
+		r := v.Pop().Bytes()
+		require.Equal(t, 9, len(r), "got: %x", r)
+
+		v, err = c.TestInvoke(t, "encodeDecode", n)
+		require.NoError(t, err)
+		require.Equal(t, 1, v.Len())
+		require.Equal(t, n, v.Pop().BigInt().Uint64())
+	}
+
+	t.Run("bad cases should be handled", func(t *testing.T) {
+		x := new(big.Int).SetUint64(math.MaxUint64)
+		x.Add(x, big.NewInt(1))
+
+		nums := []*big.Int{x, big.NewInt(-1), big.NewInt(-128)}
+		for _, n := range nums {
+			v, err := c.TestInvoke(t, "encodeDecode", n)
+			require.NoError(t, err)
+			require.Equal(t, 0, v.Pop().BigInt().Cmp(n))
+		}
+	})
+}

tests/container_test.go

@@ -3,6 +3,7 @@ package tests
 import (
 	"bytes"
 	"crypto/sha256"
+	"fmt"
 	"math/big"
 	"path"
 	"testing"
@@ -12,6 +13,7 @@ import (
 	"git.frostfs.info/TrueCloudLab/frostfs-contract/nns"
 	"github.com/mr-tron/base58"
 	"github.com/nspcc-dev/neo-go/pkg/core/interop/storage"
+	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/encoding/address"
 	"github.com/nspcc-dev/neo-go/pkg/neotest"
 	"github.com/nspcc-dev/neo-go/pkg/util"
@@ -57,6 +59,24 @@ func newContainerInvoker(t *testing.T) (*neotest.ContractInvoker, *neotest.Contr
 	return e.CommitteeInvoker(ctrContainer.Hash), e.CommitteeInvoker(ctrBalance.Hash), e.CommitteeInvoker(ctrNetmap.Hash)
 }
 
+// TODO(alexvanin): remove this after fix of inconsistent tx cost in balance contract
+func newFreeContainerInvoker(t *testing.T) (*neotest.ContractInvoker, *neotest.ContractInvoker, *neotest.ContractInvoker) {
+	e := newExecutor(t)
+
+	ctrNNS := neotest.CompileFile(t, e.CommitteeHash, nnsPath, path.Join(nnsPath, "config.yml"))
+	ctrNetmap := neotest.CompileFile(t, e.CommitteeHash, netmapPath, path.Join(netmapPath, "config.yml"))
+	ctrBalance := neotest.CompileFile(t, e.CommitteeHash, balancePath, path.Join(balancePath, "config.yml"))
+	ctrContainer := neotest.CompileFile(t, e.CommitteeHash, containerPath, path.Join(containerPath, "config.yml"))
+
+	e.DeployContract(t, ctrNNS, nil)
+	deployNetmapContract(t, e, ctrBalance.Hash, ctrContainer.Hash,
+		container.RegistrationFeeKey, int64(0),
+		container.AliasFeeKey, int64(0))
+	deployBalanceContract(t, e, ctrNetmap.Hash, ctrContainer.Hash)
+	deployContainerContract(t, e, ctrNetmap.Hash, ctrBalance.Hash, ctrNNS.Hash)
+	return e.CommitteeInvoker(ctrContainer.Hash), e.CommitteeInvoker(ctrBalance.Hash), e.CommitteeInvoker(ctrNetmap.Hash)
+}
+
 func setContainerOwner(c []byte, acc neotest.Signer) {
 	copy(c[6:], signerToOwner(acc))
 }
@@ -231,6 +251,30 @@ func TestContainerPut(t *testing.T) {
 			cNNS.Invoke(t, expected, "resolve", "domain.cdn", int64(nns.TXT))
 		})
 	})
+
+	t.Run("gas costs are the same for all containers in block", func(t *testing.T) {
+		c, _, _ := newFreeContainerInvoker(t)
+		const containerPerBlock = 512
+
+		acc := c.NewAccount(t)
+		cnt := dummyContainer(acc)
+		putArgs := []interface{}{cnt.value, cnt.sig, cnt.pub, cnt.token, "precreated", ""}
+		c.Invoke(t, stackitem.Null{}, "putNamed", putArgs...)
+
+		txs := make([]*transaction.Transaction, 0, containerPerBlock)
+		for i := 0; i < containerPerBlock; i++ {
+			cnt := dummyContainer(acc)
+			name := fmt.Sprintf("name-%.5d", i)
+			tx := c.PrepareInvoke(t, "putNamed", cnt.value, cnt.sig, cnt.pub, cnt.token, name, "")
+			txs = append(txs, tx)
+		}
+
+		c.AddNewBlock(t, txs...)
+
+		for i := 0; i < containerPerBlock; i++ {
+			c.CheckHalt(t, txs[i].Hash(), stackitem.Make(stackitem.Null{}))
+		}
+	})
 }
 
 func addContainer(t *testing.T, c, cBal *neotest.ContractInvoker) (neotest.Signer, testContainer) {
@@ -282,6 +326,30 @@ func TestContainerDelete(t *testing.T) {
 		c.InvokeFail(t, container.NotFoundError, "deletionInfo", id[:])
 	})
 
+	t.Run("gas costs are the same for different epochs", func(t *testing.T) {
+		_, cnt2 := addContainer(t, c, cBal)
+		args := []interface{}{cnt2.id[:], cnt2.sig, cnt2.pub, cnt2.token}
+
+		tx := c.PrepareInvoke(t, "delete", args...)
+		for _, e := range []int{126, 127, 128, 129, 65536} {
+			cNm.Invoke(t, stackitem.Null{}, "newEpoch", e)
+
+			// Sanity check.
+			s, err := cNm.TestInvoke(t, "epoch")
+			require.NoError(t, err)
+			require.Equal(t, big.NewInt(int64(e)), s.Top().BigInt())
+
+			tx2 := c.PrepareInvoke(t, "delete", args...)
+			require.Equal(t, tx.Size(), tx2.Size())
+			require.Equal(t, tx.SystemFee, tx2.SystemFee)
+			require.Equal(t, tx.NetworkFee, tx2.NetworkFee)
+		}
+
+		// Another sanity check: we want to test successful invocations,
+		// bad ones can trivially be equal.
+		c.Invoke(t, stackitem.Null{}, "delete", args...)
+	})
+
 	c.InvokeFail(t, container.NotFoundError, "get", cnt.id[:])
 }
 

tests/testdata/config.yml

@@ -0,0 +1,2 @@
+name: "TestContract"
+safemethods: ["encodeDecode"]

tests/testdata/encode.go

@@ -0,0 +1,17 @@
+package testdata
+
+import (
+	"git.frostfs.info/TrueCloudLab/frostfs-contract/common"
+)
+
+// EncodeDecode encodes x in fixed-width little-endian representation
+// and deserializes it back.
+func EncodeDecode(x int) int {
+	y := common.ToFixedWidth64(x)
+	return common.FromFixedWidth64(y)
+}
+
+// Encode encodes x in fixed-width little-endian representation.
+func Encode(x int) []byte {
+	return common.ToFixedWidth64(x)
+}