From 7e340725196ef983d4101c86b1edc6d871dd5f44 Mon Sep 17 00:00:00 2001 From: Evgenii Stratonikov Date: Thu, 20 Aug 2020 11:02:11 +0300 Subject: [PATCH] core: implement (*Blockchain).VerifyWitness `ScriptFromWitness` is no longer useful, because we support contract verification. --- pkg/consensus/consensus.go | 2 +- pkg/consensus/payload.go | 26 ---------- pkg/consensus/payload_test.go | 12 +++-- pkg/consensus/recovery_message_test.go | 35 +++++++------- pkg/core/blockchain.go | 67 +++++++++++++------------- pkg/core/blockchain_test.go | 21 -------- pkg/core/blockchainer/blockchainer.go | 2 + pkg/network/helper_test.go | 4 ++ 8 files changed, 67 insertions(+), 102 deletions(-) diff --git a/pkg/consensus/consensus.go b/pkg/consensus/consensus.go index 4971fd52d..544489946 100644 --- a/pkg/consensus/consensus.go +++ b/pkg/consensus/consensus.go @@ -246,7 +246,7 @@ func (s *service) validatePayload(p *Payload) bool { pub := validators[p.validatorIndex] h := pub.(*publicKey).GetScriptHash() - return p.Verify(h) + return s.Chain.VerifyWitness(h, p, &p.Witness, payloadGasLimit) == nil } func (s *service) getKeyPair(pubs []crypto.PublicKey) (int, crypto.PrivateKey, crypto.PublicKey) { diff --git a/pkg/consensus/payload.go b/pkg/consensus/payload.go index 60af7b20e..59fc57f6b 100644 --- a/pkg/consensus/payload.go +++ b/pkg/consensus/payload.go @@ -6,13 +6,9 @@ import ( "github.com/nspcc-dev/dbft/payload" "github.com/nspcc-dev/neo-go/pkg/config/netmode" - "github.com/nspcc-dev/neo-go/pkg/core" - "github.com/nspcc-dev/neo-go/pkg/core/interop" - "github.com/nspcc-dev/neo-go/pkg/core/interop/crypto" "github.com/nspcc-dev/neo-go/pkg/core/transaction" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/io" - "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/emit" ) @@ -215,28 +211,6 @@ func (p *Payload) GetSignedPart() []byte { return p.MarshalUnsigned() } -// Verify verifies payload using provided Witness. -func (p *Payload) Verify(scriptHash util.Uint160) bool { - verification, err := core.ScriptFromWitness(scriptHash, &p.Witness) - if err != nil { - return false - } - - ic := &interop.Context{Trigger: trigger.Verification, Container: p} - crypto.Register(ic) - v := ic.SpawnVM() - v.GasLimit = payloadGasLimit - v.LoadScript(verification) - v.LoadScript(p.Witness.InvocationScript) - - err = v.Run() - if err != nil || v.HasFailed() || v.Estack().Len() != 1 { - return false - } - - return v.Estack().Pop().Bool() -} - // DecodeBinaryUnsigned reads payload from w excluding signature. func (p *Payload) DecodeBinaryUnsigned(r *io.BinReader) { p.version = r.ReadU32LE() diff --git a/pkg/consensus/payload_test.go b/pkg/consensus/payload_test.go index 46ef1b797..e9ae2def2 100644 --- a/pkg/consensus/payload_test.go +++ b/pkg/consensus/payload_test.go @@ -88,7 +88,9 @@ func TestConsensusPayload_Verify(t *testing.T) { p := NewPayload(netmode.TestNet) require.NoError(t, testserdes.DecodeBinary(data, p)) require.NoError(t, p.decodeData()) - require.True(t, p.Verify(h)) + bc := newTestChain(t) + defer bc.Close() + require.NoError(t, bc.VerifyWitness(h, p, &p.Witness, payloadGasLimit)) } func TestConsensusPayload_Serializable(t *testing.T) { @@ -307,10 +309,14 @@ func TestPayload_Sign(t *testing.T) { require.NoError(t, err) priv := &privateKey{key} + p := randomPayload(t, prepareRequestType) - require.False(t, p.Verify(util.Uint160{})) + h := priv.PublicKey().GetScriptHash() + bc := newTestChain(t) + defer bc.Close() + require.Error(t, bc.VerifyWitness(h, p, &p.Witness, payloadGasLimit)) require.NoError(t, p.Sign(priv)) - require.True(t, p.Verify(p.Witness.ScriptHash())) + require.NoError(t, bc.VerifyWitness(h, p, &p.Witness, payloadGasLimit)) } func TestMessageType_String(t *testing.T) { diff --git a/pkg/consensus/recovery_message_test.go b/pkg/consensus/recovery_message_test.go index c8dedf6c3..629453085 100644 --- a/pkg/consensus/recovery_message_test.go +++ b/pkg/consensus/recovery_message_test.go @@ -9,22 +9,23 @@ import ( "github.com/nspcc-dev/dbft/payload" "github.com/nspcc-dev/neo-go/pkg/config/netmode" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" + "github.com/nspcc-dev/neo-go/pkg/internal/testchain" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/stretchr/testify/require" ) func TestRecoveryMessage_Setters(t *testing.T) { - const size = 5 - - privs := getKeys(t, size) - pubs := make([]crypto.PublicKey, 5) - for i := range pubs { - pubs[i] = &publicKey{privs[i].PublicKey()} + srv := newTestService(t) + defer srv.Chain.Close() + privs := make([]*privateKey, testchain.Size()) + pubs := make([]crypto.PublicKey, testchain.Size()) + for i := 0; i < testchain.Size(); i++ { + privs[i], pubs[i] = getTestValidator(i) } r := &recoveryMessage{} - p := new(Payload) + p := NewPayload(netmode.UnitTestNet) p.message = &message{} p.SetType(payload.RecoveryMessageType) p.SetPayload(r) @@ -36,7 +37,7 @@ func TestRecoveryMessage_Setters(t *testing.T) { nonce: 321, transactionHashes: []util.Uint256{{1}}, } - p1 := new(Payload) + p1 := NewPayload(netmode.UnitTestNet) p1.message = &message{} p1.SetType(payload.PrepareRequestType) p1.SetPayload(req) @@ -44,7 +45,7 @@ func TestRecoveryMessage_Setters(t *testing.T) { require.NoError(t, p1.Sign(privs[0])) t.Run("prepare response is added", func(t *testing.T) { - p2 := new(Payload) + p2 := NewPayload(netmode.UnitTestNet) p2.message = &message{} p2.SetType(payload.PrepareResponseType) p2.SetPayload(&prepareResponse{ @@ -61,7 +62,7 @@ func TestRecoveryMessage_Setters(t *testing.T) { require.Len(t, ps, 1) require.Equal(t, p2, ps[0]) ps0 := ps[0].(*Payload) - require.True(t, ps0.Verify(ps0.Witness.ScriptHash())) + require.True(t, srv.validatePayload(ps0)) }) t.Run("prepare request is added", func(t *testing.T) { @@ -75,11 +76,11 @@ func TestRecoveryMessage_Setters(t *testing.T) { require.Equal(t, p1, pr) pl := pr.(*Payload) - require.True(t, pl.Verify(pl.Witness.ScriptHash())) + require.True(t, srv.validatePayload(pl)) }) t.Run("change view is added", func(t *testing.T) { - p3 := new(Payload) + p3 := NewPayload(netmode.UnitTestNet) p3.message = &message{} p3.SetType(payload.ChangeViewType) p3.SetPayload(&changeView{ @@ -96,16 +97,16 @@ func TestRecoveryMessage_Setters(t *testing.T) { require.Equal(t, p3, ps[0]) ps0 := ps[0].(*Payload) - require.True(t, ps0.Verify(ps0.Witness.ScriptHash())) + require.True(t, srv.validatePayload(ps0)) }) t.Run("commit is added", func(t *testing.T) { - p4 := new(Payload) + p4 := NewPayload(netmode.UnitTestNet) p4.message = &message{} p4.SetType(payload.CommitType) p4.SetPayload(randomMessage(t, commitType)) - p4.SetValidatorIndex(4) - require.NoError(t, p4.Sign(privs[4])) + p4.SetValidatorIndex(3) + require.NoError(t, p4.Sign(privs[3])) r.AddPayload(p4) @@ -114,7 +115,7 @@ func TestRecoveryMessage_Setters(t *testing.T) { require.Equal(t, p4, ps[0]) ps0 := ps[0].(*Payload) - require.True(t, ps0.Verify(ps0.Witness.ScriptHash())) + require.True(t, srv.validatePayload(ps0)) }) } diff --git a/pkg/core/blockchain.go b/pkg/core/blockchain.go index 63685bd30..3de7cdc14 100644 --- a/pkg/core/blockchain.go +++ b/pkg/core/blockchain.go @@ -18,6 +18,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/storage" "github.com/nspcc-dev/neo-go/pkg/core/transaction" + "github.com/nspcc-dev/neo-go/pkg/crypto" "github.com/nspcc-dev/neo-go/pkg/crypto/hash" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/encoding/bigint" @@ -27,7 +28,6 @@ import ( "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/emit" "github.com/nspcc-dev/neo-go/pkg/vm/stackitem" "go.uber.org/zap" ) @@ -1373,10 +1373,7 @@ func (bc *Blockchain) verifyStateRootWitness(r *state.MPTRoot) error { if err != nil { return err } - interopCtx := bc.newInteropContext(trigger.Verification, bc.dao, nil, nil) - interopCtx.Container = r - return bc.verifyHashAgainstScript(b.NextConsensus, r.Witness, interopCtx, true, - bc.contracts.Policy.GetMaxVerificationGas(interopCtx.DAO)) + return bc.VerifyWitness(b.NextConsensus, r, r.Witness, bc.contracts.Policy.GetMaxVerificationGas(bc.dao)) } // VerifyTx verifies whether transaction is bonafide or not relative to the @@ -1439,22 +1436,6 @@ func (bc *Blockchain) GetTestVM(tx *transaction.Transaction) *vm.VM { return vm } -// ScriptFromWitness returns verification script for provided witness. -// If hash is not equal to the witness script hash, error is returned. -func ScriptFromWitness(hash util.Uint160, witness *transaction.Witness) ([]byte, error) { - verification := witness.VerificationScript - - if len(verification) == 0 { - bb := io.NewBufBinWriter() - emit.AppCall(bb.BinWriter, hash) - verification = bb.Bytes() - } else if h := witness.ScriptHash(); hash != h { - return nil, ErrWitnessHashMismatch - } - - return verification, nil -} - // Various witness verification errors. var ( ErrWitnessHashMismatch = errors.New("witness hash mismatch") @@ -1463,8 +1444,8 @@ var ( ErrInvalidVerificationContract = errors.New("verification contract is missing `verify` method") ) -// verifyHashAgainstScript verifies given hash against the given witness. -func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transaction.Witness, interopCtx *interop.Context, useKeys bool, gas int64) error { +// initVerificationVM initializes VM for witness check. +func initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transaction.Witness, keyCache map[string]*keys.PublicKey) error { var offset int var initMD *manifest.Method verification := witness.VerificationScript @@ -1473,7 +1454,7 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa return ErrWitnessHashMismatch } } else { - cs, err := interopCtx.DAO.GetContractState(hash) + cs, err := ic.DAO.GetContractState(hash) if err != nil { return ErrUnknownVerificationContract } @@ -1486,6 +1467,28 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa initMD = cs.Manifest.ABI.GetMethod(manifest.MethodInit) } + v := ic.VM + v.LoadScriptWithFlags(verification, smartcontract.NoneFlag) + v.Jump(v.Context(), offset) + if initMD != nil { + v.Call(v.Context(), initMD.Offset) + } + v.LoadScript(witness.InvocationScript) + if keyCache != nil { + v.SetPublicKeys(keyCache) + } + return nil +} + +// VerifyWitness checks that w is a correct witness for c signed by h. +func (bc *Blockchain) VerifyWitness(h util.Uint160, c crypto.Verifiable, w *transaction.Witness, gas int64) error { + ic := bc.newInteropContext(trigger.Verification, bc.dao, nil, nil) + ic.Container = c + return bc.verifyHashAgainstScript(h, w, ic, true, gas) +} + +// verifyHashAgainstScript verifies given hash against the given witness. +func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transaction.Witness, interopCtx *interop.Context, useKeys bool, gas int64) error { gasPolicy := bc.contracts.Policy.GetMaxVerificationGas(interopCtx.DAO) if gas > gasPolicy { gas = gasPolicy @@ -1494,19 +1497,17 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa vm := interopCtx.SpawnVM() vm.SetPriceGetter(getPrice) vm.GasLimit = gas - vm.LoadScriptWithFlags(verification, smartcontract.NoneFlag) - vm.Jump(vm.Context(), offset) - if initMD != nil { - vm.Call(vm.Context(), initMD.Offset) - } - vm.LoadScript(witness.InvocationScript) + var keyCache map[string]*keys.PublicKey if useKeys { bc.keyCacheLock.RLock() if bc.keyCache[hash] != nil { - vm.SetPublicKeys(bc.keyCache[hash]) + keyCache = bc.keyCache[hash] } bc.keyCacheLock.RUnlock() } + if err := initVerificationVM(interopCtx, hash, witness, keyCache); err != nil { + return err + } err := vm.Run() if vm.HasFailed() { return fmt.Errorf("%w: vm execution has failed: %v", ErrVerificationFailed, err) @@ -1564,9 +1565,7 @@ func (bc *Blockchain) verifyHeaderWitnesses(currHeader, prevHeader *block.Header } else { hash = prevHeader.NextConsensus } - interopCtx := bc.newInteropContext(trigger.Verification, bc.dao, nil, nil) - interopCtx.Container = currHeader - return bc.verifyHashAgainstScript(hash, &currHeader.Script, interopCtx, true, verificationGasLimit) + return bc.VerifyWitness(hash, currHeader, &currHeader.Script, verificationGasLimit) } // GoverningTokenHash returns the governing token (NEO) native contract hash. diff --git a/pkg/core/blockchain_test.go b/pkg/core/blockchain_test.go index 2bed8c7a7..0138d6b76 100644 --- a/pkg/core/blockchain_test.go +++ b/pkg/core/blockchain_test.go @@ -155,27 +155,6 @@ func TestAddBadBlock(t *testing.T) { require.NoError(t, bc.AddBlock(b3)) } -func TestScriptFromWitness(t *testing.T) { - witness := &transaction.Witness{} - h := util.Uint160{1, 2, 3} - - res, err := ScriptFromWitness(h, witness) - require.NoError(t, err) - require.NotNil(t, res) - - witness.VerificationScript = []byte{4, 8, 15, 16, 23, 42} - h = hash.Hash160(witness.VerificationScript) - - res, err = ScriptFromWitness(h, witness) - require.NoError(t, err) - require.NotNil(t, res) - - h[0] = ^h[0] - res, err = ScriptFromWitness(h, witness) - require.Error(t, err) - require.Nil(t, res) -} - func TestGetHeader(t *testing.T) { bc := newTestChain(t) tx := transaction.New(netmode.UnitTestNet, []byte{byte(opcode.PUSH1)}, 0) diff --git a/pkg/core/blockchainer/blockchainer.go b/pkg/core/blockchainer/blockchainer.go index dea4412b6..318ad2b72 100644 --- a/pkg/core/blockchainer/blockchainer.go +++ b/pkg/core/blockchainer/blockchainer.go @@ -8,6 +8,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/mempool" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/transaction" + "github.com/nspcc-dev/neo-go/pkg/crypto" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/util" "github.com/nspcc-dev/neo-go/pkg/vm" @@ -57,6 +58,7 @@ type Blockchainer interface { SubscribeForNotifications(ch chan<- *state.NotificationEvent) SubscribeForTransactions(ch chan<- *transaction.Transaction) VerifyTx(*transaction.Transaction) error + VerifyWitness(util.Uint160, crypto.Verifiable, *transaction.Witness, int64) error GetMemPool() *mempool.Pool UnsubscribeFromBlocks(ch chan<- *block.Block) UnsubscribeFromExecutions(ch chan<- *state.AppExecResult) diff --git a/pkg/network/helper_test.go b/pkg/network/helper_test.go index 8c8d67d3c..dd9473ed6 100644 --- a/pkg/network/helper_test.go +++ b/pkg/network/helper_test.go @@ -13,6 +13,7 @@ import ( "github.com/nspcc-dev/neo-go/pkg/core/mempool" "github.com/nspcc-dev/neo-go/pkg/core/state" "github.com/nspcc-dev/neo-go/pkg/core/transaction" + "github.com/nspcc-dev/neo-go/pkg/crypto" "github.com/nspcc-dev/neo-go/pkg/crypto/keys" "github.com/nspcc-dev/neo-go/pkg/io" "github.com/nspcc-dev/neo-go/pkg/network/capability" @@ -169,6 +170,9 @@ func (chain testChain) SubscribeForTransactions(ch chan<- *transaction.Transacti func (chain testChain) VerifyTx(*transaction.Transaction) error { panic("TODO") } +func (testChain) VerifyWitness(util.Uint160, crypto.Verifiable, *transaction.Witness, int64) error { + panic("TODO") +} func (chain testChain) UnsubscribeFromBlocks(ch chan<- *block.Block) { panic("TODO")