vm/core: drop old key caching system

Obsoleted by f5f58a7e91.

pkg/core/blockchain.go

@@ -115,11 +115,6 @@ type Blockchain struct {
 
 	memPool *mempool.Pool
 
-	// This lock protects concurrent access to keyCache.
-	keyCacheLock sync.RWMutex
-	// cache for block verification keys.
-	keyCache map[util.Uint160]map[string]*keys.PublicKey
-
 	sbCommittee keys.PublicKeys
 
 	log *zap.Logger
@@ -169,7 +164,6 @@ func NewBlockchain(s storage.Store, cfg config.ProtocolConfiguration, log *zap.L
 		stopCh:        make(chan struct{}),
 		runToExitCh:   make(chan struct{}),
 		memPool:       mempool.New(cfg.MemPoolSize),
-		keyCache:      make(map[util.Uint160]map[string]*keys.PublicKey),
 		sbCommittee:   committee,
 		log:           log,
 		events:        make(chan bcEvent),
@@ -771,8 +765,8 @@ func (bc *Blockchain) processNEP5Transfer(cache *dao.Cached, h util.Uint256, b *
 	if nativeContract != nil {
 		id = nativeContract.Metadata().ContractID
 	} else {
-		assetContract := bc.GetContractState(sc)
+		assetContract, err := cache.GetContractState(sc)
-		if assetContract == nil {
+		if err != nil {
 			return
 		}
 		id = assetContract.ID
@@ -1482,7 +1476,7 @@ var (
 )
 
 // initVerificationVM initializes VM for witness check.
-func initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transaction.Witness, keyCache map[string]*keys.PublicKey) error {
+func initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transaction.Witness) error {
 	var offset int
 	var initMD *manifest.Method
 	verification := witness.VerificationScript
@@ -1511,9 +1505,6 @@ func initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transac
 		v.Call(v.Context(), initMD.Offset)
 	}
 	v.LoadScript(witness.InvocationScript)
-	if keyCache != nil {
-		v.SetPublicKeys(keyCache)
-	}
 	return nil
 }
 
@@ -1521,11 +1512,11 @@ func initVerificationVM(ic *interop.Context, hash util.Uint160, witness *transac
 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)
+	return bc.verifyHashAgainstScript(h, w, ic, 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 {
+func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transaction.Witness, interopCtx *interop.Context, gas int64) error {
 	gasPolicy := bc.contracts.Policy.GetMaxVerificationGas(interopCtx.DAO)
 	if gas > gasPolicy {
 		gas = gasPolicy
@@ -1534,15 +1525,7 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa
 	vm := interopCtx.SpawnVM()
 	vm.SetPriceGetter(getPrice)
 	vm.GasLimit = gas
-	var keyCache map[string]*keys.PublicKey
+	if err := initVerificationVM(interopCtx, hash, witness); err != nil {
-	if useKeys {
-		bc.keyCacheLock.RLock()
-		if bc.keyCache[hash] != nil {
-			keyCache = bc.keyCache[hash]
-		}
-		bc.keyCacheLock.RUnlock()
-	}
-	if err := initVerificationVM(interopCtx, hash, witness, keyCache); err != nil {
 		return err
 	}
 	err := vm.Run()
@@ -1561,16 +1544,6 @@ func (bc *Blockchain) verifyHashAgainstScript(hash util.Uint160, witness *transa
 		if vm.Estack().Len() != 0 {
 			return fmt.Errorf("%w: expected exactly one returned value", ErrVerificationFailed)
 		}
-		if useKeys {
-			bc.keyCacheLock.RLock()
-			_, ok := bc.keyCache[hash]
-			bc.keyCacheLock.RUnlock()
-			if !ok {
-				bc.keyCacheLock.Lock()
-				bc.keyCache[hash] = vm.GetPublicKeys()
-				bc.keyCacheLock.Unlock()
-			}
-		}
 	} else {
 		return fmt.Errorf("%w: no result returned from the script", ErrVerificationFailed)
 	}
@@ -1589,7 +1562,7 @@ func (bc *Blockchain) verifyTxWitnesses(t *transaction.Transaction, block *block
 	}
 	interopCtx := bc.newInteropContext(trigger.Verification, bc.dao, block, t)
 	for i := range t.Signers {
-		err := bc.verifyHashAgainstScript(t.Signers[i].Account, &t.Scripts[i], interopCtx, false, t.NetworkFee)
+		err := bc.verifyHashAgainstScript(t.Signers[i].Account, &t.Scripts[i], interopCtx, t.NetworkFee)
 		if err != nil {
 			return fmt.Errorf("witness #%d: %w", i, err)
 		}

pkg/core/blockchain_test.go

@@ -394,22 +394,22 @@ func TestVerifyHashAgainstScript(t *testing.T) {
 			newH := cs.ScriptHash()
 			newH[0] = ^newH[0]
 			w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}}
-			err := bc.verifyHashAgainstScript(newH, w, ic, false, gas)
+			err := bc.verifyHashAgainstScript(newH, w, ic, gas)
 			require.True(t, errors.Is(err, ErrUnknownVerificationContract))
 		})
 		t.Run("Invalid", func(t *testing.T) {
 			w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}}
-			err := bc.verifyHashAgainstScript(csInvalid.ScriptHash(), w, ic, false, gas)
+			err := bc.verifyHashAgainstScript(csInvalid.ScriptHash(), w, ic, gas)
 			require.True(t, errors.Is(err, ErrInvalidVerificationContract))
 		})
 		t.Run("ValidSignature", func(t *testing.T) {
 			w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH4)}}
-			err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, false, gas)
+			err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, gas)
 			require.NoError(t, err)
 		})
 		t.Run("InvalidSignature", func(t *testing.T) {
 			w := &transaction.Witness{InvocationScript: []byte{byte(opcode.PUSH3)}}
-			err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, false, gas)
+			err := bc.verifyHashAgainstScript(cs.ScriptHash(), w, ic, gas)
 			require.True(t, errors.Is(err, ErrVerificationFailed))
 		})
 	})
@@ -419,7 +419,7 @@ func TestVerifyHashAgainstScript(t *testing.T) {
 			InvocationScript:   []byte{byte(opcode.NOP)},
 			VerificationScript: verif,
 		}
-		err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, false, 1)
+		err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, 1)
 		require.True(t, errors.Is(err, ErrVerificationFailed))
 	})
 	t.Run("NoResult", func(t *testing.T) {
@@ -428,7 +428,7 @@ func TestVerifyHashAgainstScript(t *testing.T) {
 			InvocationScript:   []byte{byte(opcode.PUSH1)},
 			VerificationScript: verif,
 		}
-		err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, false, gas)
+		err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas)
 		require.True(t, errors.Is(err, ErrVerificationFailed))
 	})
 	t.Run("BadResult", func(t *testing.T) {
@@ -439,7 +439,7 @@ func TestVerifyHashAgainstScript(t *testing.T) {
 			InvocationScript:   []byte{byte(opcode.NOP)},
 			VerificationScript: verif,
 		}
-		err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, false, gas)
+		err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas)
 		require.True(t, errors.Is(err, ErrVerificationFailed))
 	})
 	t.Run("TooManyResults", func(t *testing.T) {
@@ -448,7 +448,7 @@ func TestVerifyHashAgainstScript(t *testing.T) {
 			InvocationScript:   []byte{byte(opcode.PUSH1), byte(opcode.PUSH1)},
 			VerificationScript: verif,
 		}
-		err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, false, gas)
+		err := bc.verifyHashAgainstScript(hash.Hash160(verif), w, ic, gas)
 		require.True(t, errors.Is(err, ErrVerificationFailed))
 	})
 }

pkg/vm/vm.go

@@ -80,9 +80,6 @@ type VM struct {
 	SyscallHandler func(v *VM, id uint32) error
 
 	trigger trigger.Type
-
-	// Public keys cache.
-	keys map[string]*keys.PublicKey
 }
 
 // New returns a new VM object ready to load AVM bytecode scripts.
@@ -96,7 +93,6 @@ func NewWithTrigger(t trigger.Type) *VM {
 		state:   NoneState,
 		istack:  NewStack("invocation"),
 		refs:    newRefCounter(),
-		keys:    make(map[string]*keys.PublicKey),
 		trigger: t,
 
 		SyscallHandler: defaultSyscallHandler,
@@ -140,17 +136,6 @@ func (v *VM) Istack() *Stack {
 	return v.istack
 }
 
-// SetPublicKeys sets internal key cache to the specified value (note
-// that it doesn't copy them).
-func (v *VM) SetPublicKeys(keys map[string]*keys.PublicKey) {
-	v.keys = keys
-}
-
-// GetPublicKeys returns internal key cache (note that it doesn't copy it).
-func (v *VM) GetPublicKeys() map[string]*keys.PublicKey {
-	return v.keys
-}
-
 // LoadArgs loads in the arguments used in the Mian entry point.
 func (v *VM) LoadArgs(method []byte, args []stackitem.Item) {
 	if len(args) > 0 {
@@ -1586,8 +1571,8 @@ func CheckMultisigPar(v *VM, curve elliptic.Curve, h []byte, pkeys [][]byte, sig
 		go worker(tasks, results)
 	}
 
-	tasks <- task{pub: v.bytesToPublicKey(pkeys[k1], curve), signum: s1}
+	tasks <- task{pub: bytesToPublicKey(pkeys[k1], curve), signum: s1}
-	tasks <- task{pub: v.bytesToPublicKey(pkeys[k2], curve), signum: s2}
+	tasks <- task{pub: bytesToPublicKey(pkeys[k2], curve), signum: s2}
 
 	sigok := true
 	taskCount := 2
@@ -1631,7 +1616,7 @@ loop:
 			nextKey = k2
 		}
 		taskCount++
-		tasks <- task{pub: v.bytesToPublicKey(pkeys[nextKey], curve), signum: nextSig}
+		tasks <- task{pub: bytesToPublicKey(pkeys[nextKey], curve), signum: nextSig}
 	}
 
 	close(tasks)
@@ -1641,7 +1626,7 @@ loop:
 
 func checkMultisig1(v *VM, curve elliptic.Curve, h []byte, pkeys [][]byte, sig []byte) bool {
 	for i := range pkeys {
-		pkey := v.bytesToPublicKey(pkeys[i], curve)
+		pkey := bytesToPublicKey(pkeys[i], curve)
 		if pkey.Verify(sig, h) {
 			return true
 		}
@@ -1696,19 +1681,11 @@ func (v *VM) checkInvocationStackSize() {
 
 // bytesToPublicKey is a helper deserializing keys using cache and panicing on
 // error.
-func (v *VM) bytesToPublicKey(b []byte, curve elliptic.Curve) *keys.PublicKey {
+func bytesToPublicKey(b []byte, curve elliptic.Curve) *keys.PublicKey {
-	var pkey *keys.PublicKey
+	pkey, err := keys.NewPublicKeyFromBytes(b, curve)
-	s := string(b)
-	if v.keys[s] != nil {
-		pkey = v.keys[s]
-	} else {
-		var err error
-		pkey, err = keys.NewPublicKeyFromBytes(b, curve)
 	if err != nil {
 		panic(err.Error())
 	}
-		v.keys[s] = pkey
-	}
 	return pkey
 }
 

pkg/vm/vm_test.go

@@ -2,7 +2,6 @@ package vm
 
 import (
 	"bytes"
-	"crypto/elliptic"
 	"encoding/binary"
 	"encoding/hex"
 	"errors"
@@ -103,25 +102,6 @@ func TestAddGas(t *testing.T) {
 	require.False(t, v.AddGas(5))
 }
 
-func TestBytesToPublicKey(t *testing.T) {
-	v := newTestVM()
-	cache := v.GetPublicKeys()
-	assert.Equal(t, 0, len(cache))
-	keyHex := "03b209fd4f53a7170ea4444e0cb0a6bb6a53c2bd016926989cf85f9b0fba17a70c"
-	keyBytes, _ := hex.DecodeString(keyHex)
-	key := v.bytesToPublicKey(keyBytes, elliptic.P256())
-	assert.NotNil(t, key)
-	key2 := v.bytesToPublicKey(keyBytes, elliptic.P256())
-	assert.Equal(t, key, key2)
-
-	cache = v.GetPublicKeys()
-	assert.Equal(t, 1, len(cache))
-	assert.NotNil(t, cache[string(keyBytes)])
-
-	keyBytes[0] = 0xff
-	require.Panics(t, func() { v.bytesToPublicKey(keyBytes, elliptic.P256()) })
-}
-
 func TestPushBytes1to75(t *testing.T) {
 	buf := io.NewBufBinWriter()
 	for i := 1; i <= 75; i++ {