Merge pull request #1695 from nspcc-dev/committeechecks

native: unify committee checks

pkg/core/native/management.go

@@ -91,7 +91,7 @@ func newManagement() *Management {
 	md = newMethodAndPrice(m.getMinimumDeploymentFee, 100_0000, callflag.ReadStates)
 	m.AddMethod(md, desc)
 
-	desc = newDescriptor("setMinimumDeploymentFee", smartcontract.BoolType,
+	desc = newDescriptor("setMinimumDeploymentFee", smartcontract.VoidType,
 		manifest.NewParameter("value", smartcontract.IntegerType))
 	md = newMethodAndPrice(m.setMinimumDeploymentFee, 300_0000, callflag.WriteStates)
 	m.AddMethod(md, desc)
@@ -369,13 +369,13 @@ func (m *Management) setMinimumDeploymentFee(ic *interop.Context, args []stackit
 		panic(fmt.Errorf("MinimumDeploymentFee cannot be negative"))
 	}
 	if !m.NEO.checkCommittee(ic) {
-		return stackitem.NewBool(false)
+		panic("invalid committee signature")
 	}
 	err := setUint32WithKey(m.ContractID, ic.DAO, keyMinimumDeploymentFee, value)
 	if err != nil {
 		panic(err)
 	}
-	return stackitem.NewBool(true)
+	return stackitem.Null{}
 }
 
 func (m *Management) callDeploy(ic *interop.Context, cs *state.Contract, isUpdate bool) {

pkg/core/native/native_neo.go

@@ -145,7 +145,7 @@ func newNEO() *NEO {
 	md = newMethodAndPrice(n.getGASPerBlock, 100_0000, callflag.ReadStates)
 	n.AddMethod(md, desc)
 
-	desc = newDescriptor("setGasPerBlock", smartcontract.BoolType,
+	desc = newDescriptor("setGasPerBlock", smartcontract.VoidType,
 		manifest.NewParameter("gasPerBlock", smartcontract.IntegerType))
 	md = newMethodAndPrice(n.setGASPerBlock, 500_0000, callflag.WriteStates)
 	n.AddMethod(md, desc)
@@ -472,25 +472,23 @@ func (n *NEO) checkCommittee(ic *interop.Context) bool {
 
 func (n *NEO) setGASPerBlock(ic *interop.Context, args []stackitem.Item) stackitem.Item {
 	gas := toBigInt(args[0])
-	ok, err := n.SetGASPerBlock(ic, ic.Block.Index+1, gas)
+	err := n.SetGASPerBlock(ic, ic.Block.Index+1, gas)
 	if err != nil {
 		panic(err)
 	}
-	return stackitem.NewBool(ok)
+	return stackitem.Null{}
 }
 
 // SetGASPerBlock sets gas generated for blocks after index.
-func (n *NEO) SetGASPerBlock(ic *interop.Context, index uint32, gas *big.Int) (bool, error) {
+func (n *NEO) SetGASPerBlock(ic *interop.Context, index uint32, gas *big.Int) error {
 	if gas.Sign() == -1 || gas.Cmp(big.NewInt(10*GASFactor)) == 1 {
-		return false, errors.New("invalid value for GASPerBlock")
+		return errors.New("invalid value for GASPerBlock")
 	}
-	h := n.GetCommitteeAddress()
+	if !n.checkCommittee(ic) {
-	ok, err := runtime.CheckHashedWitness(ic, h)
+		return errors.New("invalid committee signature")
-	if err != nil || !ok {
-		return ok, err
 	}
 	n.gasPerBlockChanged.Store(true)
-	return true, n.putGASRecord(ic.DAO, index, gas)
+	return n.putGASRecord(ic.DAO, index, gas)
 }
 
 func (n *NEO) dropCandidateIfZero(d dao.DAO, pub *keys.PublicKey, c *candidate) (bool, error) {

pkg/core/native/notary.go

@@ -93,7 +93,7 @@ func newNotary() *Notary {
 	md = newMethodAndPrice(n.getMaxNotValidBeforeDelta, 100_0000, callflag.ReadStates)
 	n.AddMethod(md, desc)
 
-	desc = newDescriptor("setMaxNotValidBeforeDelta", smartcontract.BoolType,
+	desc = newDescriptor("setMaxNotValidBeforeDelta", smartcontract.VoidType,
 		manifest.NewParameter("value", smartcontract.IntegerType))
 	md = newMethodAndPrice(n.setMaxNotValidBeforeDelta, 300_0000, callflag.WriteStates)
 	n.AddMethod(md, desc)
@@ -389,7 +389,7 @@ func (n *Notary) setMaxNotValidBeforeDelta(ic *interop.Context, args []stackitem
 		panic(fmt.Errorf("MaxNotValidBeforeDelta cannot be more than %d or less than %d", transaction.MaxValidUntilBlockIncrement/2, ic.Chain.GetConfig().ValidatorsCount))
 	}
 	if !n.NEO.checkCommittee(ic) {
-		return stackitem.NewBool(false)
+		panic("invalid committee signature")
 	}
 	n.lock.Lock()
 	defer n.lock.Unlock()
@@ -398,7 +398,7 @@ func (n *Notary) setMaxNotValidBeforeDelta(ic *interop.Context, args []stackitem
 		panic(fmt.Errorf("failed to put value into the storage: %w", err))
 	}
 	n.isValid = false
-	return stackitem.NewBool(true)
+	return stackitem.Null{}
 }
 
 // GetDepositFor returns state.Deposit for the account specified. It returns nil in case if

pkg/core/native/policy.go

@@ -109,7 +109,7 @@ func newPolicy() *Policy {
 	md = newMethodAndPrice(p.getExecFeeFactor, 1000000, callflag.ReadStates)
 	p.AddMethod(md, desc)
 
-	desc = newDescriptor("setExecFeeFactor", smartcontract.BoolType,
+	desc = newDescriptor("setExecFeeFactor", smartcontract.VoidType,
 		manifest.NewParameter("value", smartcontract.IntegerType))
 	md = newMethodAndPrice(p.setExecFeeFactor, 3000000, callflag.WriteStates)
 	p.AddMethod(md, desc)
@@ -118,27 +118,27 @@ func newPolicy() *Policy {
 	md = newMethodAndPrice(p.getStoragePrice, 1000000, callflag.ReadStates)
 	p.AddMethod(md, desc)
 
-	desc = newDescriptor("setStoragePrice", smartcontract.BoolType,
+	desc = newDescriptor("setStoragePrice", smartcontract.VoidType,
 		manifest.NewParameter("value", smartcontract.IntegerType))
 	md = newMethodAndPrice(p.setStoragePrice, 3000000, callflag.WriteStates)
 	p.AddMethod(md, desc)
 
-	desc = newDescriptor("setMaxBlockSize", smartcontract.BoolType,
+	desc = newDescriptor("setMaxBlockSize", smartcontract.VoidType,
 		manifest.NewParameter("value", smartcontract.IntegerType))
 	md = newMethodAndPrice(p.setMaxBlockSize, 3000000, callflag.WriteStates)
 	p.AddMethod(md, desc)
 
-	desc = newDescriptor("setMaxTransactionsPerBlock", smartcontract.BoolType,
+	desc = newDescriptor("setMaxTransactionsPerBlock", smartcontract.VoidType,
 		manifest.NewParameter("value", smartcontract.IntegerType))
 	md = newMethodAndPrice(p.setMaxTransactionsPerBlock, 3000000, callflag.WriteStates)
 	p.AddMethod(md, desc)
 
-	desc = newDescriptor("setFeePerByte", smartcontract.BoolType,
+	desc = newDescriptor("setFeePerByte", smartcontract.VoidType,
 		manifest.NewParameter("value", smartcontract.IntegerType))
 	md = newMethodAndPrice(p.setFeePerByte, 3000000, callflag.WriteStates)
 	p.AddMethod(md, desc)
 
-	desc = newDescriptor("setMaxBlockSystemFee", smartcontract.BoolType,
+	desc = newDescriptor("setMaxBlockSystemFee", smartcontract.VoidType,
 		manifest.NewParameter("value", smartcontract.IntegerType))
 	md = newMethodAndPrice(p.setMaxBlockSystemFee, 3000000, callflag.WriteStates)
 	p.AddMethod(md, desc)
@@ -309,7 +309,7 @@ func (p *Policy) setExecFeeFactor(ic *interop.Context, args []stackitem.Item) st
 		panic(fmt.Errorf("ExecFeeFactor must be between 0 and %d", maxExecFeeFactor))
 	}
 	if !p.NEO.checkCommittee(ic) {
-		return stackitem.NewBool(false)
+		panic("invalid committee signature")
 	}
 	p.lock.Lock()
 	defer p.lock.Unlock()
@@ -318,7 +318,7 @@ func (p *Policy) setExecFeeFactor(ic *interop.Context, args []stackitem.Item) st
 		panic(err)
 	}
 	p.isValid = false
-	return stackitem.NewBool(true)
+	return stackitem.Null{}
 }
 
 // isBlocked is Policy contract method and checks whether provided account is blocked.
@@ -365,7 +365,7 @@ func (p *Policy) setStoragePrice(ic *interop.Context, args []stackitem.Item) sta
 		panic(fmt.Errorf("StoragePrice must be between 0 and %d", maxStoragePrice))
 	}
 	if !p.NEO.checkCommittee(ic) {
-		return stackitem.NewBool(false)
+		panic("invalid committee signature")
 	}
 	p.lock.Lock()
 	defer p.lock.Unlock()
@@ -374,7 +374,7 @@ func (p *Policy) setStoragePrice(ic *interop.Context, args []stackitem.Item) sta
 		panic(err)
 	}
 	p.isValid = false
-	return stackitem.NewBool(true)
+	return stackitem.Null{}
 }
 
 // setMaxTransactionsPerBlock is Policy contract method and  sets the upper limit
@@ -385,7 +385,7 @@ func (p *Policy) setMaxTransactionsPerBlock(ic *interop.Context, args []stackite
 		panic(fmt.Errorf("MaxTransactionsPerBlock cannot exceed the maximum allowed transactions per block = %d", block.MaxTransactionsPerBlock))
 	}
 	if !p.NEO.checkCommittee(ic) {
-		return stackitem.NewBool(false)
+		panic("invalid committee signature")
 	}
 	p.lock.Lock()
 	defer p.lock.Unlock()
@@ -394,7 +394,7 @@ func (p *Policy) setMaxTransactionsPerBlock(ic *interop.Context, args []stackite
 		panic(err)
 	}
 	p.isValid = false
-	return stackitem.NewBool(true)
+	return stackitem.Null{}
 }
 
 // setMaxBlockSize is Policy contract method and sets maximum block size.
@@ -404,7 +404,7 @@ func (p *Policy) setMaxBlockSize(ic *interop.Context, args []stackitem.Item) sta
 		panic(fmt.Errorf("MaxBlockSize cannot be more than the maximum payload size = %d", payload.MaxSize))
 	}
 	if !p.NEO.checkCommittee(ic) {
-		return stackitem.NewBool(false)
+		panic("invalid committee signature")
 	}
 	p.lock.Lock()
 	defer p.lock.Unlock()
@@ -413,7 +413,7 @@ func (p *Policy) setMaxBlockSize(ic *interop.Context, args []stackitem.Item) sta
 		panic(err)
 	}
 	p.isValid = false
-	return stackitem.NewBool(true)
+	return stackitem.Null{}
 }
 
 // setFeePerByte is Policy contract method and sets transaction's fee per byte.
@@ -423,7 +423,7 @@ func (p *Policy) setFeePerByte(ic *interop.Context, args []stackitem.Item) stack
 		panic(fmt.Errorf("FeePerByte shouldn't be negative or greater than %d", maxFeePerByte))
 	}
 	if !p.NEO.checkCommittee(ic) {
-		return stackitem.NewBool(false)
+		panic("invalid committee signature")
 	}
 	p.lock.Lock()
 	defer p.lock.Unlock()
@@ -432,7 +432,7 @@ func (p *Policy) setFeePerByte(ic *interop.Context, args []stackitem.Item) stack
 		panic(err)
 	}
 	p.isValid = false
-	return stackitem.NewBool(true)
+	return stackitem.Null{}
 }
 
 // setMaxBlockSystemFee is Policy contract method and sets the maximum system fee per block.
@@ -442,7 +442,7 @@ func (p *Policy) setMaxBlockSystemFee(ic *interop.Context, args []stackitem.Item
 		panic(fmt.Errorf("MaxBlockSystemFee cannot be less then %d", minBlockSystemFee))
 	}
 	if !p.NEO.checkCommittee(ic) {
-		return stackitem.NewBool(false)
+		panic("invalid committee signature")
 	}
 	p.lock.Lock()
 	defer p.lock.Unlock()
@@ -451,14 +451,14 @@ func (p *Policy) setMaxBlockSystemFee(ic *interop.Context, args []stackitem.Item
 		panic(err)
 	}
 	p.isValid = false
-	return stackitem.NewBool(true)
+	return stackitem.Null{}
 }
 
 // blockAccount is Policy contract method and adds given account hash to the list
 // of blocked accounts.
 func (p *Policy) blockAccount(ic *interop.Context, args []stackitem.Item) stackitem.Item {
 	if !p.NEO.checkCommittee(ic) {
-		return stackitem.NewBool(false)
+		panic("invalid committee signature")
 	}
 	hash := toUint160(args[0])
 	if p.IsBlockedInternal(ic.DAO, hash) {
@@ -481,7 +481,7 @@ func (p *Policy) blockAccount(ic *interop.Context, args []stackitem.Item) stacki
 // the list of blocked accounts.
 func (p *Policy) unblockAccount(ic *interop.Context, args []stackitem.Item) stackitem.Item {
 	if !p.NEO.checkCommittee(ic) {
-		return stackitem.NewBool(false)
+		panic("invalid committee signature")
 	}
 	hash := toUint160(args[0])
 	if !p.IsBlockedInternal(ic.DAO, hash) {

pkg/core/native_neo_test.go

@@ -197,57 +197,8 @@ func TestNEO_SetGasPerBlock(t *testing.T) {
 	bc := newTestChain(t)
 	defer bc.Close()
 
-	neo := bc.contracts.NEO
+	testGetSet(t, bc, bc.contracts.NEO.Hash, "GasPerBlock",
-	tx := transaction.New(netmode.UnitTestNet, []byte{}, 0)
+		5*native.GASFactor, 0, 10*native.GASFactor)
-	ic := bc.newInteropContext(trigger.Application, bc.dao, nil, tx)
-	ic.SpawnVM()
-	ic.VM.LoadScript([]byte{byte(opcode.RET)})
-
-	h := neo.GetCommitteeAddress()
-	t.Run("Default", func(t *testing.T) {
-		g := neo.GetGASPerBlock(ic.DAO, 0)
-		require.EqualValues(t, 5*native.GASFactor, g.Int64())
-	})
-	t.Run("Invalid", func(t *testing.T) {
-		t.Run("InvalidSignature", func(t *testing.T) {
-			setSigner(tx, util.Uint160{})
-			ok, err := neo.SetGASPerBlock(ic, 10, big.NewInt(native.GASFactor))
-			require.NoError(t, err)
-			require.False(t, ok)
-		})
-		t.Run("TooBigValue", func(t *testing.T) {
-			setSigner(tx, h)
-			_, err := neo.SetGASPerBlock(ic, 10, big.NewInt(10*native.GASFactor+1))
-			require.Error(t, err)
-		})
-	})
-	t.Run("Valid", func(t *testing.T) {
-		setSigner(tx, h)
-		ok, err := neo.SetGASPerBlock(ic, 10, big.NewInt(native.GASFactor*2))
-		require.NoError(t, err)
-		require.True(t, ok)
-		_, err = ic.DAO.Persist()
-		require.NoError(t, err)
-
-		t.Run("Again", func(t *testing.T) {
-			setSigner(tx, h)
-			ok, err := neo.SetGASPerBlock(ic, 10, big.NewInt(native.GASFactor))
-			require.NoError(t, err)
-			require.True(t, ok)
-
-			t.Run("NotPersisted", func(t *testing.T) {
-				g := neo.GetGASPerBlock(bc.dao, 10)
-				// Old value should be returned.
-				require.EqualValues(t, 2*native.GASFactor, g.Int64())
-			})
-		})
-
-		g := neo.GetGASPerBlock(ic.DAO, 9)
-		require.EqualValues(t, 5*native.GASFactor, g.Int64())
-
-		g = neo.GetGASPerBlock(ic.DAO, 10)
-		require.EqualValues(t, native.GASFactor, g.Int64())
-	})
 }
 
 func TestNEO_CalculateBonus(t *testing.T) {
@@ -270,9 +221,8 @@ func TestNEO_CalculateBonus(t *testing.T) {
 	})
 	t.Run("ManyBlocks", func(t *testing.T) {
 		setSigner(tx, neo.GetCommitteeAddress())
-		ok, err := neo.SetGASPerBlock(ic, 10, big.NewInt(1*native.GASFactor))
+		err := neo.SetGASPerBlock(ic, 10, big.NewInt(1*native.GASFactor))
 		require.NoError(t, err)
-		require.True(t, ok)
 
 		res, err := neo.CalculateNEOHolderReward(ic.DAO, big.NewInt(100), 5, 15)
 		require.NoError(t, err)

pkg/core/native_notary_test.go

@@ -2,14 +2,12 @@ package core
 
 import (
 	"math"
-	"math/big"
 	"testing"
 
 	"github.com/nspcc-dev/neo-go/internal/testchain"
 	"github.com/nspcc-dev/neo-go/pkg/core/native"
 	"github.com/nspcc-dev/neo-go/pkg/core/transaction"
 	"github.com/nspcc-dev/neo-go/pkg/crypto/keys"
-	"github.com/nspcc-dev/neo-go/pkg/encoding/bigint"
 	"github.com/nspcc-dev/neo-go/pkg/io"
 	"github.com/nspcc-dev/neo-go/pkg/smartcontract/callflag"
 	"github.com/nspcc-dev/neo-go/pkg/smartcontract/trigger"
@@ -329,46 +327,7 @@ func TestNotaryNodesReward(t *testing.T) {
 func TestMaxNotValidBeforeDelta(t *testing.T) {
 	chain := newTestChain(t)
 	defer chain.Close()
-	notaryHash := chain.contracts.Notary.Hash
 
-	transferFundsToCommittee(t, chain)
+	testGetSet(t, chain, chain.contracts.Notary.Hash, "MaxNotValidBeforeDelta",
-
+		140, int64(chain.GetConfig().ValidatorsCount), transaction.MaxValidUntilBlockIncrement/2)
-	t.Run("get, internal method", func(t *testing.T) {
-		n := chain.contracts.Notary.GetMaxNotValidBeforeDelta(chain.dao)
-		require.Equal(t, 140, int(n))
-	})
-
-	t.Run("get, contract method", func(t *testing.T) {
-		res, err := invokeContractMethod(chain, 100000000, notaryHash, "getMaxNotValidBeforeDelta")
-		require.NoError(t, err)
-		checkResult(t, res, stackitem.NewBigInteger(big.NewInt(140)))
-		require.NoError(t, chain.persist())
-	})
-
-	t.Run("set", func(t *testing.T) {
-		res, err := invokeContractMethodGeneric(chain, 100000000, notaryHash, "setMaxNotValidBeforeDelta", true, bigint.ToBytes(big.NewInt(150)))
-		require.NoError(t, err)
-		checkResult(t, res, stackitem.NewBool(true))
-		n := chain.contracts.Notary.GetMaxNotValidBeforeDelta(chain.dao)
-		require.Equal(t, 150, int(n))
-	})
-
-	t.Run("set, too big value", func(t *testing.T) {
-		res, err := invokeContractMethodGeneric(chain, 100000000, notaryHash, "setMaxNotValidBeforeDelta", true, bigint.ToBytes(big.NewInt(transaction.MaxValidUntilBlockIncrement/2+1)))
-		require.NoError(t, err)
-		checkFAULTState(t, res)
-	})
-
-	t.Run("set, too small value", func(t *testing.T) {
-		res, err := invokeContractMethodGeneric(chain, 100000000, notaryHash, "setMaxNotValidBeforeDelta", true, bigint.ToBytes(big.NewInt(int64(chain.GetConfig().ValidatorsCount-1))))
-		require.NoError(t, err)
-		checkFAULTState(t, res)
-	})
-
-	t.Run("set, not signed by committee", func(t *testing.T) {
-		signer, err := wallet.NewAccount()
-		require.NoError(t, err)
-		invokeRes, err := invokeContractMethodBy(t, chain, signer, notaryHash, "setMaxNotValidBeforeDelta", bigint.ToBytes(big.NewInt(150)))
-		checkResult(t, invokeRes, stackitem.NewBool(false))
-	})
 }

pkg/core/native_policy_test.go

@@ -29,7 +29,8 @@ func testGetSet(t *testing.T, chain *Blockchain, hash util.Uint160, name string,
 		signer, err := wallet.NewAccount()
 		require.NoError(t, err)
 		invokeRes, err := invokeContractMethodBy(t, chain, signer, hash, setName, minValue+1)
-		checkResult(t, invokeRes, stackitem.NewBool(false))
+		require.NoError(t, err)
+		checkFAULTState(t, invokeRes)
 	})
 
 	t.Run("get, defult value", func(t *testing.T) {
@@ -61,8 +62,10 @@ func testGetSet(t *testing.T, chain *Blockchain, hash util.Uint160, name string,
 		require.NoError(t, err)
 		aers, err := persistBlock(chain, txSet, txGet1)
 		require.NoError(t, err)
-		checkResult(t, aers[0], stackitem.NewBool(true))
+		checkResult(t, aers[0], stackitem.Null{})
-		checkResult(t, aers[1], stackitem.Make(defaultValue+1))
+		if name != "GasPerBlock" { // GasPerBlock is set on the next block
+			checkResult(t, aers[1], stackitem.Make(defaultValue+1))
+		}
 		require.NoError(t, chain.persist())
 
 		// Get in the next block.
@@ -214,6 +217,11 @@ func TestBlockedAccounts(t *testing.T) {
 		signer, err := wallet.NewAccount()
 		require.NoError(t, err)
 		invokeRes, err := invokeContractMethodBy(t, chain, signer, policyHash, "blockAccount", account.BytesBE())
-		checkResult(t, invokeRes, stackitem.NewBool(false))
+		require.NoError(t, err)
+		checkFAULTState(t, invokeRes)
+
+		invokeRes, err = invokeContractMethodBy(t, chain, signer, policyHash, "unblockAccount", account.BytesBE())
+		require.NoError(t, err)
+		checkFAULTState(t, invokeRes)
 	})
 }