Merge pull request #1747 from nspcc-dev/default-notary-config

notary: usability improvements

config/protocol.mainnet.yml

@@ -20,6 +20,7 @@ ProtocolConfiguration:
   - seed5.neo.org:10333
   VerifyBlocks: true
   VerifyTransactions: false
+  P2PSigExtensions: false
 
 ApplicationConfiguration:
   # LogPath could be set up in case you need stdout logs to some proper file.
@@ -50,6 +51,11 @@ ApplicationConfiguration:
   MinPeers: 5
   Oracle:
     Enabled: false
+  P2PNotary:
+    Enabled: false
+    UnlockWallet:
+      Path: "/notary_wallet.json"
+      Password: "pass"
 
   RPC:
     Enabled: true

config/protocol.privnet.docker.four.yml

@@ -16,6 +16,7 @@ ProtocolConfiguration:
     - 172.200.0.4:20336
   VerifyBlocks: true
   VerifyTransactions: true
+  P2PSigExtensions: false
 
 ApplicationConfiguration:
   # LogPath could be set up in case you need stdout logs to some proper file.
@@ -55,6 +56,11 @@ ApplicationConfiguration:
     UnlockWallet:
       Path: "/wallet4.json"
       Password: "four"
+  P2PNotary:
+    Enabled: false
+    UnlockWallet:
+      Path: "/notary_wallet.json"
+      Password: "pass"
   RPC:
     Enabled: true
     MaxGasInvoke: 15

config/protocol.privnet.docker.one.yml

@@ -16,6 +16,7 @@ ProtocolConfiguration:
     - 172.200.0.4:20336
   VerifyBlocks: true
   VerifyTransactions: true
+  P2PSigExtensions: false
 
 ApplicationConfiguration:
   # LogPath could be set up in case you need stdout logs to some proper file.
@@ -55,6 +56,11 @@ ApplicationConfiguration:
     UnlockWallet:
       Path: "/wallet1.json"
       Password: "one"
+  P2PNotary:
+    Enabled: false
+    UnlockWallet:
+      Path: "/notary_wallet.json"
+      Password: "pass"
   RPC:
     Enabled: true
     MaxGasInvoke: 15

config/protocol.privnet.docker.single.yml

@@ -10,6 +10,7 @@ ProtocolConfiguration:
     - 172.200.0.1:20333
   VerifyBlocks: true
   VerifyTransactions: true
+  P2PSigExtensions: false
 
 ApplicationConfiguration:
   # LogPath could be set up in case you need stdout logs to some proper file.
@@ -46,6 +47,11 @@ ApplicationConfiguration:
     UnlockWallet:
       Path: "/wallet1_solo.json"
       Password: "one"
+  P2PNotary:
+    Enabled: false
+    UnlockWallet:
+      Path: "/notary_wallet.json"
+      Password: "pass"
   RPC:
     Enabled: true
     EnableCORSWorkaround: false

config/protocol.privnet.docker.three.yml

@@ -16,6 +16,7 @@ ProtocolConfiguration:
     - 172.200.0.4:20336
   VerifyBlocks: true
   VerifyTransactions: true
+  P2PSigExtensions: false
 
 ApplicationConfiguration:
   # LogPath could be set up in case you need stdout logs to some proper file.
@@ -55,6 +56,11 @@ ApplicationConfiguration:
     UnlockWallet:
       Path: "/wallet3.json"
       Password: "three"
+  P2PNotary:
+    Enabled: false
+    UnlockWallet:
+      Path: "/notary_wallet.json"
+      Password: "pass"
   RPC:
     Enabled: true
     MaxGasInvoke: 15

config/protocol.privnet.docker.two.yml

@@ -16,6 +16,7 @@ ProtocolConfiguration:
     - 172.200.0.4:20336
   VerifyBlocks: true
   VerifyTransactions: true
+  P2PSigExtensions: false
 
 ApplicationConfiguration:
   # LogPath could be set up in case you need stdout logs to some proper file.
@@ -55,6 +56,11 @@ ApplicationConfiguration:
     UnlockWallet:
       Path: "/wallet2.json"
       Password: "two"
+  P2PNotary:
+    Enabled: false
+    UnlockWallet:
+      Path: "/notary_wallet.json"
+      Password: "pass"
   RPC:
     Enabled: true
     MaxGasInvoke: 15

config/protocol.privnet.yml

@@ -16,6 +16,7 @@ ProtocolConfiguration:
     - 127.0.0.1:20336
   VerifyBlocks: true
   VerifyTransactions: true
+  P2PSigExtensions: false
 
 ApplicationConfiguration:
   # LogPath could be set up in case you need stdout logs to some proper file.
@@ -44,6 +45,11 @@ ApplicationConfiguration:
   MaxPeers: 10
   AttemptConnPeers: 5
   MinPeers: 3
+  P2PNotary:
+    Enabled: false
+    UnlockWallet:
+      Path: "/notary_wallet.json"
+      Password: "pass"
   RPC:
     Enabled: true
     MaxGasInvoke: 15

config/protocol.testnet.yml

@@ -20,6 +20,7 @@ ProtocolConfiguration:
   - seed5t.neo.org:20333
   VerifyBlocks: true
   VerifyTransactions: false
+  P2PSigExtensions: false
 
 ApplicationConfiguration:
   # LogPath could be set up in case you need stdout logs to some proper file.
@@ -50,7 +51,11 @@ ApplicationConfiguration:
   MinPeers: 5
   Oracle:
     Enabled: false
-
+  P2PNotary:
+    Enabled: false
+    UnlockWallet:
+      Path: "/notary_wallet.json"
+      Password: "pass"
   RPC:
     Enabled: true
     MaxGasInvoke: 15

config/protocol.unit_testnet.single.yml

@@ -8,6 +8,7 @@ ProtocolConfiguration:
   ValidatorsCount: 1
   VerifyBlocks: true
   VerifyTransactions: true
+  P2PSigExtensions: false
 
 ApplicationConfiguration:
   # LogPath could be set up in case you need stdout logs to some proper file.
@@ -39,6 +40,11 @@ ApplicationConfiguration:
   UnlockWallet:
       Path: "testdata/wallet1_solo.json"
       Password: "one"
+  P2PNotary:
+    Enabled: false
+    UnlockWallet:
+      Path: "/notary_wallet.json"
+      Password: "pass"
   RPC:
     Address: 127.0.0.1
     MaxGasInvoke: 15

config/protocol.unit_testnet.yml

@@ -46,6 +46,11 @@ ApplicationConfiguration:
   MaxPeers: 50
   AttemptConnPeers: 5
   MinPeers: 1
+  P2PNotary:
+    Enabled: false
+    UnlockWallet:
+      Path: "/notary_wallet.json"
+      Password: "pass"
   RPC:
     Address: 127.0.0.1
     MaxGasInvoke: 15

pkg/config/application_config.go

@@ -27,4 +27,5 @@ type ApplicationConfiguration struct {
 	RPC               rpc.Config              `yaml:"RPC"`
 	UnlockWallet      Wallet                  `yaml:"UnlockWallet"`
 	Oracle            OracleConfiguration     `yaml:"Oracle"`
+	P2PNotary         P2PNotary               `yaml:"P2PNotary"`
 }

pkg/config/protocol_config.go

@@ -20,8 +20,6 @@ type (
 		RemoveUntraceableBlocks bool `yaml:"RemoveUntraceableBlocks"`
 		// MaxTraceableBlocks is the length of the chain accessible to smart contracts.
 		MaxTraceableBlocks uint32 `yaml:"MaxTraceableBlocks"`
-		// P2PNotary stores configuration for P2P notary node service
-		P2PNotary P2PNotary `yaml:"P2PNotary"`
 		// P2PSigExtensions enables additional signature-related logic.
 		P2PSigExtensions bool `yaml:"P2PSigExtensions"`
 		// ReservedAttributes allows to have reserved attributes range for experimental or private purposes.

pkg/core/notary_test.go

@@ -34,15 +34,20 @@ import (
 const notaryModulePath = "../services/notary/"
 
 func getTestNotary(t *testing.T, bc *Blockchain, walletPath, pass string, onTx func(tx *transaction.Transaction) error) (*wallet.Account, *notary.Notary, *mempool.Pool) {
-	bc.config.P2PNotary = config.P2PNotary{
+	mainCfg := config.P2PNotary{
 		Enabled: true,
 		UnlockWallet: config.Wallet{
 			Path:     path.Join(notaryModulePath, walletPath),
 			Password: pass,
 		},
 	}
+	cfg := notary.Config{
+		MainCfg: mainCfg,
+		Chain:   bc,
+		Log:     zaptest.NewLogger(t),
+	}
 	mp := mempool.New(10, 1, true)
-	ntr, err := notary.NewNotary(bc, mp, zaptest.NewLogger(t), onTx)
+	ntr, err := notary.NewNotary(cfg, mp, onTx)
 	require.NoError(t, err)
 
 	w, err := wallet.NewWalletFromFile(path.Join(notaryModulePath, walletPath))

pkg/network/relay_reason.go

@@ -1,5 +1,7 @@
 package network
 
+//go:generate stringer -type=RelayReason -output=relay_reason_string.go
+
 // RelayReason is the type which describes the different relay outcome.
 type RelayReason uint8
 

pkg/network/relay_reason_string.go

@@ -0,0 +1,29 @@
+// Code generated by "stringer -type=RelayReason -output=relay_reason_string.go"; DO NOT EDIT.
+
+package network
+
+import "strconv"
+
+func _() {
+	// An "invalid array index" compiler error signifies that the constant values have changed.
+	// Re-run the stringer command to generate them again.
+	var x [1]struct{}
+	_ = x[RelaySucceed-0]
+	_ = x[RelayAlreadyExists-1]
+	_ = x[RelayOutOfMemory-2]
+	_ = x[RelayUnableToVerify-3]
+	_ = x[RelayInvalid-4]
+	_ = x[RelayPolicyFail-5]
+	_ = x[RelayUnknown-6]
+}
+
+const _RelayReason_name = "RelaySucceedRelayAlreadyExistsRelayOutOfMemoryRelayUnableToVerifyRelayInvalidRelayPolicyFailRelayUnknown"
+
+var _RelayReason_index = [...]uint8{0, 12, 30, 46, 65, 77, 92, 104}
+
+func (i RelayReason) String() string {
+	if i >= RelayReason(len(_RelayReason_index)-1) {
+		return "RelayReason(" + strconv.FormatInt(int64(i), 10) + ")"
+	}
+	return _RelayReason_name[_RelayReason_index[i]:_RelayReason_index[i+1]]
+}

pkg/network/server.go

@@ -139,17 +139,22 @@ func newServerFromConstructors(config ServerConfig, chain blockchainer.Blockchai
 	}
 	if chain.P2PSigExtensionsEnabled() {
 		s.notaryFeer = NewNotaryFeer(chain)
-		s.notaryRequestPool = mempool.New(chain.GetConfig().P2PNotaryRequestPayloadPoolSize, 1, chain.GetConfig().P2PNotary.Enabled)
+		s.notaryRequestPool = mempool.New(chain.GetConfig().P2PNotaryRequestPayloadPoolSize, 1, config.P2PNotaryCfg.Enabled)
 		chain.RegisterPostBlock(func(bc blockchainer.Blockchainer, txpool *mempool.Pool, _ *block.Block) {
 			s.notaryRequestPool.RemoveStale(func(t *transaction.Transaction) bool {
 				return bc.IsTxStillRelevant(t, txpool, true)
 			}, s.notaryFeer)
 		})
-		if chain.GetConfig().P2PNotary.Enabled {
+		if config.P2PNotaryCfg.Enabled {
-			n, err := notary.NewNotary(chain, s.notaryRequestPool, s.log, func(tx *transaction.Transaction) error {
+			cfg := notary.Config{
+				MainCfg: config.P2PNotaryCfg,
+				Chain:   chain,
+				Log:     log,
+			}
+			n, err := notary.NewNotary(cfg, s.notaryRequestPool, func(tx *transaction.Transaction) error {
 				r := s.RelayTxn(tx)
 				if r != RelaySucceed {
-					return fmt.Errorf("can't pool notary tx: hash %s, reason: %d", tx.Hash().StringLE(), byte(r))
+					return fmt.Errorf("can't relay completed notary transaction: hash %s, reason: %s", tx.Hash().StringLE(), r.String())
 				}
 				return nil
 			})
@@ -159,7 +164,7 @@ func newServerFromConstructors(config ServerConfig, chain blockchainer.Blockchai
 			s.notaryModule = n
 			chain.SetNotary(n)
 		}
-	} else if chain.GetConfig().P2PNotary.Enabled {
+	} else if config.P2PNotaryCfg.Enabled {
 		return nil, errors.New("P2PSigExtensions are disabled, but Notary service is enable")
 	}
 	s.bQueue = newBlockQueue(maxBlockBatch, chain, log, func(b *block.Block) {

pkg/network/server_config.go

@@ -69,6 +69,9 @@ type (
 
 		// OracleCfg is oracle module configuration.
 		OracleCfg config.OracleConfiguration
+
+		// P2PNotaryCfg is notary module configuration.
+		P2PNotaryCfg config.P2PNotary
 	}
 )
 
@@ -100,5 +103,6 @@ func NewServerConfig(cfg config.Config) ServerConfig {
 		Wallet:            wc,
 		TimePerBlock:      time.Duration(protoConfig.SecondsPerBlock) * time.Second,
 		OracleCfg:         appConfig.Oracle,
+		P2PNotaryCfg:      appConfig.P2PNotary,
 	}
 }

pkg/rpc/client/rpc.go

@@ -573,12 +573,13 @@ func getSigners(sender util.Uint160, cosigners []transaction.Signer) []transacti
 // Main transaction should be constructed by the user. Several rules need to be met for
 // successful main transaction acceptance:
 // 1. Native Notary contract should be a signer of the main transaction.
-// 2. Main transaction should have dummy contract witness for Notary signer.
+// 2. Notary signer should have None scope.
-// 3. Main transaction should have NotaryAssisted attribute with NKeys specified.
+// 3. Main transaction should have dummy contract witness for Notary signer.
-// 4. NotaryAssisted attribute and dummy Notary witness (as long as the other incomplete witnesses)
+// 4. Main transaction should have NotaryAssisted attribute with NKeys specified.
+// 5. NotaryAssisted attribute and dummy Notary witness (as long as the other incomplete witnesses)
 //    should be paid for. Use CalculateNotaryWitness to calculate the amount of network fee to pay
 //    for the attribute and Notary witness.
-// 5. Main transaction either shouldn't have all witnesses attached (in this case none of them
+// 6. Main transaction either shouldn't have all witnesses attached (in this case none of them
 //	  can be multisignature), or it only should have a partial multisignature.
 // Note: client should be initialized before SignAndPushP2PNotaryRequest call.
 func (c *Client) SignAndPushP2PNotaryRequest(mainTx *transaction.Transaction, fallbackScript []byte, fallbackSysFee int64, fallbackNetFee int64, fallbackValidFor uint32, acc *wallet.Account) (*payload.P2PNotaryRequest, error) {

pkg/rpc/server/server_helper_test.go

@@ -39,7 +39,7 @@ func getUnitTestChain(t *testing.T, enableOracle bool, enableNotary bool) (*core
 	if enableNotary {
 		cfg.ProtocolConfiguration.P2PSigExtensions = true
 		cfg.ProtocolConfiguration.P2PNotaryRequestPayloadPoolSize = 1000
-		cfg.ProtocolConfiguration.P2PNotary = config.P2PNotary{
+		cfg.ApplicationConfiguration.P2PNotary = config.P2PNotary{
 			Enabled: true,
 			UnlockWallet: config.Wallet{
 				Path:     notaryPath,
@@ -47,7 +47,7 @@ func getUnitTestChain(t *testing.T, enableOracle bool, enableNotary bool) (*core
 			},
 		}
 	} else {
-		cfg.ProtocolConfiguration.P2PNotary.Enabled = false
+		cfg.ApplicationConfiguration.P2PNotary.Enabled = false
 	}
 	chain, err := core.NewBlockchain(memoryStore, cfg.ProtocolConfiguration, logger)
 	require.NoError(t, err, "could not create chain")

pkg/services/notary/node_test.go

@@ -14,7 +14,7 @@ import (
 )
 
 func getTestNotary(t *testing.T, bc blockchainer.Blockchainer, walletPath, pass string) (*wallet.Account, *Notary, *mempool.Pool) {
-	bc.(*fakechain.FakeChain).ProtocolConfiguration.P2PNotary = config.P2PNotary{
+	mainCfg := config.P2PNotary{
 		Enabled: true,
 		UnlockWallet: config.Wallet{
 			Path:     walletPath,
@@ -22,7 +22,12 @@ func getTestNotary(t *testing.T, bc blockchainer.Blockchainer, walletPath, pass
 		},
 	}
 	mp := mempool.New(10, 1, true)
-	ntr, err := NewNotary(bc, mp, zaptest.NewLogger(t), nil)
+	cfg := Config{
+		MainCfg: mainCfg,
+		Chain:   bc,
+		Log:     zaptest.NewLogger(t),
+	}
+	ntr, err := NewNotary(cfg, mp, nil)
 	require.NoError(t, err)
 
 	w, err := wallet.NewWalletFromFile(walletPath)

pkg/services/notary/notary.go

@@ -80,9 +80,8 @@ type request struct {
 }
 
 // NewNotary returns new Notary module.
-func NewNotary(bc blockchainer.Blockchainer, mp *mempool.Pool, log *zap.Logger, onTransaction func(tx *transaction.Transaction) error) (*Notary, error) {
+func NewNotary(cfg Config, mp *mempool.Pool, onTransaction func(tx *transaction.Transaction) error) (*Notary, error) {
-	cfg := bc.GetConfig().P2PNotary
+	w := cfg.MainCfg.UnlockWallet
-	w := cfg.UnlockWallet
 	wallet, err := wallet.NewWalletFromFile(w.Path)
 	if err != nil {
 		return nil, err
@@ -101,11 +100,7 @@ func NewNotary(bc blockchainer.Blockchainer, mp *mempool.Pool, log *zap.Logger,
 
 	return &Notary{
 		requests:      make(map[util.Uint256]*request),
-		Config: Config{
+		Config:        cfg,
-			MainCfg: cfg,
-			Chain:   bc,
-			Log:     log,
-		},
 		wallet:        wallet,
 		onTransaction: onTransaction,
 		mp:            mp,

pkg/services/notary/notary_test.go

@@ -19,40 +19,29 @@ import (
 
 func TestWallet(t *testing.T) {
 	bc := fakechain.NewFakeChain()
-
+	mainCfg := config.P2PNotary{Enabled: true}
-	t.Run("unexisting wallet", func(t *testing.T) {
+	cfg := Config{
-		bc.ProtocolConfiguration.P2PNotary = config.P2PNotary{
+		MainCfg: mainCfg,
-			Enabled: true,
+		Chain:   bc,
-			UnlockWallet: config.Wallet{
+		Log:     zaptest.NewLogger(t),
-				Path:     "./testdata/does_not_exists.json",
-				Password: "one",
-			},
 	}
-		_, err := NewNotary(bc, mempool.New(1, 1, true), zaptest.NewLogger(t), nil)
+	t.Run("unexisting wallet", func(t *testing.T) {
+		cfg.MainCfg.UnlockWallet.Path = "./testdata/does_not_exists.json"
+		_, err := NewNotary(cfg, mempool.New(1, 1, true), nil)
 		require.Error(t, err)
 	})
 
 	t.Run("bad password", func(t *testing.T) {
-		bc.ProtocolConfiguration.P2PNotary = config.P2PNotary{
+		cfg.MainCfg.UnlockWallet.Path = "./testdata/notary1.json"
-			Enabled: true,
+		cfg.MainCfg.UnlockWallet.Password = "invalid"
-			UnlockWallet: config.Wallet{
+		_, err := NewNotary(cfg, mempool.New(1, 1, true), nil)
-				Path:     "./testdata/notary1.json",
-				Password: "invalid",
-			},
-		}
-		_, err := NewNotary(bc, mempool.New(1, 1, true), zaptest.NewLogger(t), nil)
 		require.Error(t, err)
 	})
 
 	t.Run("good", func(t *testing.T) {
-		bc.ProtocolConfiguration.P2PNotary = config.P2PNotary{
+		cfg.MainCfg.UnlockWallet.Path = "./testdata/notary1.json"
-			Enabled: true,
+		cfg.MainCfg.UnlockWallet.Password = "one"
-			UnlockWallet: config.Wallet{
+		_, err := NewNotary(cfg, mempool.New(1, 1, true), nil)
-				Path:     "./testdata/notary1.json",
-				Password: "one",
-			},
-		}
-		_, err := NewNotary(bc, mempool.New(1, 1, true), zaptest.NewLogger(t), nil)
 		require.NoError(t, err)
 	})
 }