fix/Notary request tracking

CHANGELOG.md

@@ -14,6 +14,7 @@ Changelog for NeoFS Node
 - `neo-go` RPC connection loss handling (#1337)
 - Concurrent morph cache misses (#1248)
 - Double voting for validators on IR startup (#2365)
+- Skip unexpected notary events on notary request parsing step (#2315)
 
 ### Removed
 - Deprecated `morph.rpc_endpoint` SN and `morph.endpoint.client` IR config sections (#2400)

pkg/morph/event/listener.go

@@ -101,7 +101,7 @@ type listener struct {
  notificationHandlers map[scriptHashWithType][]Handler
 
  listenNotary bool
- notaryEventsPreparator NotaryPreparator
+ notaryEventsPreparator preparator
  notaryParsers map[notaryRequestTypes]NotaryParser
  notaryHandlers map[notaryRequestTypes]Handler
  notaryMainTXSigner util.Uint160 // filter for notary subscription
@@ -328,7 +328,7 @@ func (l *listener) parseAndHandleNotary(nr *result.NotaryRequestEvent) {
  notaryEvent, err := l.notaryEventsPreparator.Prepare(nr.NotaryRequest)
  if err != nil {
  switch {
- case errors.Is(err, ErrTXAlreadyHandled):
+ case errors.Is(err, ErrTXAlreadyHandled) || errors.Is(err, ErrUnknownEvent):
  case errors.Is(err, ErrMainTXExpired):
  l.log.Warn("skip expired main TX notary event",
  zap.String("error", err.Error()),
@@ -515,6 +515,8 @@ func (l *listener) SetNotaryParser(pi NotaryParserInfo) {
  l.notaryParsers[pi.notaryRequestTypes] = pi.parser()
  }
 
+ l.notaryEventsPreparator.allowNotaryEvent(pi.notaryScriptWithHash)
+
  log.Info("registered new event parser")
 }
 

pkg/morph/event/notary_preparator.go

@@ -5,7 +5,9 @@ import (
  "encoding/binary"
  "errors"
  "fmt"
+ "sync"
 
+ lru "github.com/hashicorp/golang-lru/v2"
  "github.com/nspcc-dev/neo-go/pkg/core/interop/interopnames"
  "github.com/nspcc-dev/neo-go/pkg/core/transaction"
  "github.com/nspcc-dev/neo-go/pkg/crypto/hash"
@@ -42,6 +44,9 @@ var (
 
  // ErrMainTXExpired is returned if received fallback TX is already valid.
  ErrMainTXExpired = errors.New("received main tx has expired")
+
+ // ErrUnknownEvent is returned if main transaction is not expected.
+ ErrUnknownEvent = errors.New("received notary request is not allowed")
 )
 
 // BlockCounter must return block count of the network
@@ -50,7 +55,7 @@ type BlockCounter interface {
  BlockCount() (res uint32, err error)
 }
 
-// PreparatorPrm groups the required parameters of the Preparator constructor.
+// PreparatorPrm groups the required parameters of the preparator constructor.
 type PreparatorPrm struct {
  AlphaKeys client.AlphabetKeys
 
@@ -59,8 +64,9 @@ type PreparatorPrm struct {
  BlockCounter BlockCounter
 }
 
-// Preparator implements NotaryPreparator interface.
-type Preparator struct {
+// preparator constructs NotaryEvent
+// from the NotaryRequest event.
+type preparator struct {
  // contractSysCall contract call in NeoVM
  contractSysCall []byte
  // dummyInvocationScript is invocation script from TX that is not signed.
@@ -69,13 +75,21 @@ type Preparator struct {
  alphaKeys client.AlphabetKeys
 
  blockCounter BlockCounter
+
+ // cache for TX recursion; size limited because it is
+ // just an optimization, already handled TX are not gonna
+ // be signed once more anyway
+ alreadyHandledTXs *lru.Cache[util.Uint256, struct{}]
+
+ m *sync.RWMutex
+ allowedEvents map[notaryScriptWithHash]struct{}
 }
 
-// notaryPreparator inits and returns NotaryPreparator.
+// notaryPreparator inits and returns preparator.
 //
 // Considered to be used for preparing notary request
 // for parsing it by event.Listener.
-func notaryPreparator(prm PreparatorPrm) NotaryPreparator {
+func notaryPreparator(prm PreparatorPrm) preparator {
  switch {
  case prm.AlphaKeys == nil:
  panic("alphabet keys source must not be nil")
@@ -88,11 +102,17 @@ func notaryPreparator(prm PreparatorPrm) NotaryPreparator {
 
  dummyInvocationScript := append([]byte{byte(opcode.PUSHDATA1), 64}, make([]byte, 64)...)
 
- return Preparator{
+ const txCacheSize = 1000
+ cache, _ := lru.New[util.Uint256, struct{}](txCacheSize)
+
+ return preparator{
  contractSysCall: contractSysCall,
  dummyInvocationScript: dummyInvocationScript,
  alphaKeys: prm.AlphaKeys,
  blockCounter: prm.BlockCounter,
+ alreadyHandledTXs: cache,
+ m: &sync.RWMutex{},
+ allowedEvents: make(map[notaryScriptWithHash]struct{}),
  }
 }
 
@@ -103,7 +123,15 @@ func notaryPreparator(prm PreparatorPrm) NotaryPreparator {
 // transaction is expected to be received one more time
 // from the Notary service but already signed. This happens
 // since every notary call is a new notary request in fact.
-func (p Preparator) Prepare(nr *payload.P2PNotaryRequest) (NotaryEvent, error) {
+//
+// Returns ErrUnknownEvent if main transactions is not an
+// expected contract call.
+func (p preparator) Prepare(nr *payload.P2PNotaryRequest) (NotaryEvent, error) {
+ if _, ok := p.alreadyHandledTXs.Get(nr.MainTransaction.Hash()); ok {
+ // received already signed and sent TX
+ return nil, ErrTXAlreadyHandled
+ }
+
  // notary request's main tx is expected to have
  // three or four witnesses: one for proxy contract,
  // one for alphabet multisignature, one optional for
@@ -192,6 +220,18 @@ func (p Preparator) Prepare(nr *payload.P2PNotaryRequest) (NotaryEvent, error) {
 
  // retrieve contract's method
  contractMethod := string(ops[opsLen-3].param)
+ eventType := NotaryTypeFromString(contractMethod)
+
+ p.m.RLock()
+ _, allowed := p.allowedEvents[notaryScriptWithHash{
+ scriptHashValue: scriptHashValue{contractHash},
+ notaryRequestType: notaryRequestType{eventType},
+ }]
+ p.m.RUnlock()
+
+ if !allowed {
+ return nil, ErrUnknownEvent
+ }
 
  // check if there is a call flag(must be in range [0:15))
  callFlag := callflag.CallFlag(ops[opsLen-4].code - opcode.PUSH0)
@@ -211,15 +251,24 @@ func (p Preparator) Prepare(nr *payload.P2PNotaryRequest) (NotaryEvent, error) {
  args = args[:len(args)-2]
  }
 
+ p.alreadyHandledTXs.Add(nr.MainTransaction.Hash(), struct{}{})
+
  return parsedNotaryEvent{
  hash: contractHash,
- notaryType: NotaryTypeFromString(contractMethod),
+ notaryType: eventType,
  params: args,
  raw: nr,
  }, nil
 }
 
-func (p Preparator) validateParameterOpcodes(ops []Op) error {
+func (p preparator) allowNotaryEvent(filter notaryScriptWithHash) {
+ p.m.Lock()
+ defer p.m.Unlock()
+
+ p.allowedEvents[filter] = struct{}{}
+}
+
+func (p preparator) validateParameterOpcodes(ops []Op) error {
  l := len(ops)
 
  if ops[l-1].code != opcode.PACK {
@@ -283,7 +332,7 @@ func validateNestedArgs(expArgLen int64, ops []Op) error {
  return nil
 }
 
-func (p Preparator) validateExpiration(fbTX *transaction.Transaction) error {
+func (p preparator) validateExpiration(fbTX *transaction.Transaction) error {
  if len(fbTX.Attributes) != 3 {
  return errIncorrectFBAttributesAmount
  }
@@ -310,7 +359,7 @@ func (p Preparator) validateExpiration(fbTX *transaction.Transaction) error {
  return nil
 }
 
-func (p Preparator) validateCosigners(expected int, s []transaction.Signer, alphaKeys keys.PublicKeys) error {
+func (p preparator) validateCosigners(expected int, s []transaction.Signer, alphaKeys keys.PublicKeys) error {
  if len(s) != expected {
  return errUnexpectedCosignersAmount
  }
@@ -327,7 +376,7 @@ func (p Preparator) validateCosigners(expected int, s []transaction.Signer, alph
  return nil
 }
 
-func (p Preparator) validateWitnesses(w []transaction.Witness, alphaKeys keys.PublicKeys, invokerWitness bool) error {
+func (p preparator) validateWitnesses(w []transaction.Witness, alphaKeys keys.PublicKeys, invokerWitness bool) error {
  // the first one(proxy contract) must have empty
  // witnesses
  if len(w[0].VerificationScript)+len(w[0].InvocationScript) != 0 {
@@ -363,7 +412,7 @@ func (p Preparator) validateWitnesses(w []transaction.Witness, alphaKeys keys.Pu
  return nil
 }
 
-func (p Preparator) validateAttributes(aa []transaction.Attribute, alphaKeys keys.PublicKeys, invokerWitness bool) error {
+func (p preparator) validateAttributes(aa []transaction.Attribute, alphaKeys keys.PublicKeys, invokerWitness bool) error {
  // main tx must have exactly one attribute
  if len(aa) != 1 {
  return errIncorrectAttributesAmount

pkg/morph/event/notary_preparator_test.go

@@ -2,6 +2,7 @@ package event
 
 import (
  "fmt"
+ "strconv"
  "testing"
 
  "github.com/nspcc-dev/neo-go/pkg/vm"
@@ -21,6 +22,8 @@ import (
  "github.com/stretchr/testify/require"
 )
 
+const contractMethod = "test"
+
 var (
  alphaKeys keys.PublicKeys
  wrongAlphaKeys keys.PublicKeys
@@ -63,12 +66,17 @@ func TestPrepare_IncorrectScript(t *testing.T) {
  },
  )
 
+ preparator.allowNotaryEvent(notaryScriptWithHash{
+ notaryRequestType: notaryRequestType{contractMethod},
+ scriptHashValue: scriptHashValue{scriptHash},
+ })
+
  for _, dummyMultisig := range []bool{true, false} { // try both empty and dummy multisig/Notary invocation witness script
  t.Run(fmt.Sprintf("not contract call, compat: %t", dummyMultisig), func(t *testing.T) {
  bw := io.NewBufBinWriter()
 
  emit.Int(bw.BinWriter, 4)
- emit.String(bw.BinWriter, "test")
+ emit.String(bw.BinWriter, contractMethod)
  emit.Bytes(bw.BinWriter, scriptHash.BytesBE())
  emit.Syscall(bw.BinWriter, interopnames.SystemContractCallNative) // any != interopnames.SystemContractCall
 
@@ -83,7 +91,7 @@ func TestPrepare_IncorrectScript(t *testing.T) {
  bw := io.NewBufBinWriter()
 
  emit.Int(bw.BinWriter, -1)
- emit.String(bw.BinWriter, "test")
+ emit.String(bw.BinWriter, contractMethod)
  emit.Bytes(bw.BinWriter, scriptHash.BytesBE())
  emit.Syscall(bw.BinWriter, interopnames.SystemContractCall)
 
@@ -186,7 +194,13 @@ func TestPrepare_IncorrectNR(t *testing.T) {
  name: "incorrect main TX attribute amount",
  addW: false,
  mTX: mTX{
- attrs: []transaction.Attribute{{}, {}},
+ attrs: []transaction.Attribute{{
+ Type: transaction.NotValidBeforeT,
+ Value: &transaction.NotaryAssisted{},
+ }, {
+ Type: transaction.NotValidBeforeT,
+ Value: &transaction.NotaryAssisted{},
+ }},
  },
  expErr: errIncorrectAttributesAmount,
  },
@@ -196,6 +210,7 @@ func TestPrepare_IncorrectNR(t *testing.T) {
  mTX: mTX{
  attrs: []transaction.Attribute{
  {
+ Type: transaction.NotaryAssistedT,
  Value: &transaction.NotaryAssisted{
  NKeys: uint8(len(alphaKeys) + 1),
  },
@@ -371,6 +386,7 @@ func TestPrepare_IncorrectNR(t *testing.T) {
  mTX: mTX{
  attrs: []transaction.Attribute{
  {
+ Type: transaction.NotaryAssistedT,
  Value: &transaction.NotaryAssisted{
  NKeys: uint8(len(alphaKeys) + 2),
  },
@@ -440,14 +456,20 @@ func TestPrepare_CorrectNR(t *testing.T) {
 
  for _, test := range tests {
  for i := 0; i < 1; i++ { // run tests against 3 and 4 witness NR
- for _, dummyMultisig := range []bool{true, false} { // run tests against empty and dummy multisig/Notary witness
+ for j, dummyMultisig := range []bool{true, false} { // run tests against empty and dummy multisig/Notary witness
+ method := test.method + strconv.FormatInt(int64(j), 10)
+ preparator.allowNotaryEvent(notaryScriptWithHash{
+ notaryRequestType: notaryRequestType{NotaryTypeFromString(method)},
+ scriptHashValue: scriptHashValue{scriptHash},
+ })
+
  additionalWitness := i == 0
- nr := correctNR(script(test.hash, test.method, test.args...), dummyMultisig, additionalWitness)
+ nr := correctNR(script(test.hash, method, test.args...), dummyMultisig, additionalWitness)
 
  event, err := preparator.Prepare(nr)
 
  require.NoError(t, err)
- require.Equal(t, test.method, event.Type().String())
+ require.Equal(t, method, event.Type().String())
  require.Equal(t, test.hash.StringLE(), event.ScriptHash().StringLE())
 
  // check args parsing
@@ -480,6 +502,20 @@ func TestPrepare_CorrectNR(t *testing.T) {
  }
 }
 
+func TestNotAllowedEvents(t *testing.T) {
+ preparator := notaryPreparator(
+ PreparatorPrm{
+ alphaKeysSource(),
+ blockCounter{100, nil},
+ },
+ )
+
+ nr := correctNR(script(scriptHash, "test1", nil), false, false)
+
+ _, err := preparator.Prepare(nr)
+ require.ErrorIs(t, err, ErrUnknownEvent)
+}
+
 func alphaKeysSource() client.AlphabetKeys {
  return func() (keys.PublicKeys, error) {
  return alphaKeys, nil
@@ -546,6 +582,7 @@ func correctNR(script []byte, dummyMultisig, additionalWitness bool) *payload.P2
  Scripts: scripts,
  Attributes: []transaction.Attribute{
  {
+ Type: transaction.NotaryAssistedT,
  Value: &transaction.NotaryAssisted{
  NKeys: nKeys,
  },

pkg/morph/event/parsers.go

@@ -4,7 +4,6 @@ import (
  "fmt"
 
  "github.com/nspcc-dev/neo-go/pkg/core/state"
- "github.com/nspcc-dev/neo-go/pkg/network/payload"
 )
 
 // NotificationParser is a function that constructs Event
@@ -20,12 +19,6 @@ type NotificationParserInfo struct {
  p NotificationParser
 }
 
-// NotaryPreparator constructs NotaryEvent
-// from the NotaryRequest event.
-type NotaryPreparator interface {
- Prepare(*payload.P2PNotaryRequest) (NotaryEvent, error)
-}
-
 // NotaryParser is a function that constructs Event
 // from the NotaryEvent event.
 type NotaryParser func(NotaryEvent) (Event, error)

pkg/morph/event/utils.go

@@ -28,6 +28,10 @@ type scriptHashWithType struct {
 
 type notaryRequestTypes struct {
  notaryRequestMempoolType
+ notaryScriptWithHash
+}
+
+type notaryScriptWithHash struct {
  notaryRequestType
  scriptHashValue
 }