config: replace VerifyBlocks with SkipBlockVerification

It directly affects node security and the default here MUST BE the safe choice
which is to do the verification. Otherwise it's just dangerous, absent any
VerifyBlocks configuration we'll get an insecure node. This option is not
supposed to be frequently used and it doesn't affect the ability to process
blocks, so breaking compatibility (in a safe manner) should be OK here.
This commit is contained in:
Roman Khimov 2022-12-06 18:13:40 +03:00
parent d92f35664b
commit 4e7cee4e12
20 changed files with 28 additions and 32 deletions

View file

@ -151,7 +151,9 @@ release).
GarbageCollectionPeriod, KeepOnlyLatestState, RemoveUntraceableBlocks,
SaveStorageBatch and VerifyBlocks settings were moved from
ProtocolConfiguration to ApplicationConfiguration in version 0.100.0. Old
configurations are still supported.
configurations are still supported, except for VerifyBlocks which is replaced
by SkipBlockVerification with inverted meaning (and hence an inverted default)
for security reasons.
Removal of these options from ProtocolConfiguration is scheduled for May-June
2023 (~0.103.0 release).

View file

@ -37,7 +37,7 @@ ProtocolConfiguration:
OracleContract: [0]
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -49,7 +49,7 @@ ProtocolConfiguration:
OracleContract: [0]
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -28,7 +28,7 @@ ProtocolConfiguration:
OracleContract: [0]
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -28,7 +28,7 @@ ProtocolConfiguration:
OracleContract: [0]
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -22,7 +22,7 @@ ProtocolConfiguration:
OracleContract: [0]
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -28,7 +28,7 @@ ProtocolConfiguration:
OracleContract: [0]
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -28,7 +28,7 @@ ProtocolConfiguration:
OracleContract: [0]
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -28,7 +28,7 @@ ProtocolConfiguration:
OracleContract: [0]
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -37,7 +37,7 @@ ProtocolConfiguration:
Notary: [0]
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -52,7 +52,7 @@ ProtocolConfiguration:
OracleContract: [0]
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -23,7 +23,7 @@ ProtocolConfiguration:
Aspidochelone: 25
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -32,7 +32,7 @@ ProtocolConfiguration:
Aspidochelone: 25
ApplicationConfiguration:
VerifyBlocks: true
SkipBlockVerification: false
# LogPath could be set up in case you need stdout logs to some proper file.
# LogPath: "./log/neogo.log"
DBConfiguration:

View file

@ -43,9 +43,9 @@ node-related settings described in the table below.
| RemoveUntraceableBlocks | `bool`| `false` | Denotes whether old blocks should be removed from cache and database. If enabled, then only the last `MaxTraceableBlocks` are stored and accessible to smart contracts. Old MPT data is also deleted in accordance with `GarbageCollectionPeriod` setting. If enabled along with `P2PStateExchangeExtensions` protocol extension, then old blocks and MPT states will be removed up to the second latest state synchronisation point (see `StateSyncInterval`). |
| RPC | [RPC Configuration](#RPC-Configuration) | | Describes [RPC subsystem](rpc.md) configuration. See the [RPC Configuration](#RPC-Configuration) for details. |
| SaveStorageBatch | `bool` | `false` | Enables storage batch saving before every persist. It is similar to StorageDump plugin for C# node. |
| SkipBlockVerification | `bool` | `false` | Allows to disable verification of received/processed blocks (including cryptographic checks). |
| StateRoot | [State Root Configuration](#State-Root-Configuration) | | State root module configuration. See the [State Root Configuration](#State-Root-Configuration) section for details. |
| UnlockWallet | [Unlock Wallet Configuration](#Unlock-Wallet-Configuration) | | Node wallet configuration used for consensus (dBFT) operation. See the [Unlock Wallet Configuration](#Unlock-Wallet-Configuration) section for details. This section is deprecated and replaced by Consensus, it only exists for compatibility with old configuration files, but will be removed in future node versions. |
| VerifyBlocks | `bool` | `false` | Denotes whether to verify the received blocks. |
### P2P Configuration
@ -365,5 +365,5 @@ protocol-related settings described in the table below.
| TimePerBlock | `Duration` | `15s` | Minimal (and targeted for) time interval between blocks. Must be an integer number of milliseconds. |
| ValidatorsCount | `int` | `0` | Number of validators set for the whole network lifetime, can't be set if `ValidatorsHistory` setting is used. |
| ValidatorsHistory | map[uint32]int | none | Number of consensus nodes to use after given height (see `CommitteeHistory` also). Heights where the change occurs must be divisible by the number of committee members at that height. Can't be used with `ValidatorsCount` not equal to zero. |
| VerifyBlocks | `bool` | `false` | Denotes whether to verify the received blocks. | This setting is deprecated in favor of the same setting in the ApplicationConfiguration and will be removed in future node versions. If both settings are used, setting any of them to true enables the function. |
| VerifyBlocks | `bool` | `false` | This setting is deprecated and no longer works, please use `SkipBlockVerification` in the `ApplicationConfiguration`, it will be removed in future node versions. |
| VerifyTransactions | `bool` | `false` | Denotes whether to verify transactions in the received blocks. |

View file

@ -16,8 +16,9 @@ type Ledger struct {
RemoveUntraceableBlocks bool `yaml:"RemoveUntraceableBlocks"`
// SaveStorageBatch enables storage batch saving before every persist.
SaveStorageBatch bool `yaml:"SaveStorageBatch"`
// VerifyBlocks controls block verification checks (including cryptography).
VerifyBlocks bool `yaml:"VerifyBlocks"`
// SkipBlockVerification allows to disable verification of received
// blocks (including cryptographic checks).
SkipBlockVerification bool `yaml:"SkipBlockVerification"`
}
// Blockchain is a set of settings for core.Blockchain to use, it includes protocol

View file

@ -296,7 +296,6 @@ func NewBlockchain(s storage.Store, cfg config.Blockchain, log *zap.Logger) (*Bl
cfg.Ledger.KeepOnlyLatestState = cfg.Ledger.KeepOnlyLatestState || cfg.ProtocolConfiguration.KeepOnlyLatestState //nolint:staticcheck // SA1019: cfg.ProtocolConfiguration.KeepOnlyLatestState is deprecated
cfg.Ledger.RemoveUntraceableBlocks = cfg.Ledger.RemoveUntraceableBlocks || cfg.ProtocolConfiguration.RemoveUntraceableBlocks //nolint:staticcheck // SA1019: cfg.ProtocolConfiguration.RemoveUntraceableBlocks is deprecated
cfg.Ledger.SaveStorageBatch = cfg.Ledger.SaveStorageBatch || cfg.ProtocolConfiguration.SaveStorageBatch //nolint:staticcheck // SA1019: cfg.ProtocolConfiguration.SaveStorageBatch is deprecated
cfg.Ledger.VerifyBlocks = cfg.Ledger.VerifyBlocks || cfg.ProtocolConfiguration.VerifyBlocks //nolint:staticcheck // SA1019: cfg.ProtocolConfiguration.VerifyBlocks is deprecated
// Local config consistency checks.
if cfg.Ledger.RemoveUntraceableBlocks && cfg.Ledger.GarbageCollectionPeriod == 0 {
@ -1309,12 +1308,12 @@ func (bc *Blockchain) AddBlock(block *block.Block) error {
}
if block.Index == bc.HeaderHeight()+1 {
err := bc.addHeaders(bc.config.Ledger.VerifyBlocks, &block.Header)
err := bc.addHeaders(!bc.config.SkipBlockVerification, &block.Header)
if err != nil {
return err
}
}
if bc.config.Ledger.VerifyBlocks {
if !bc.config.SkipBlockVerification {
merkle := block.ComputeMerkleRoot()
if !block.MerkleRoot.Equals(merkle) {
return errors.New("invalid block: MerkleRoot mismatch")
@ -1344,7 +1343,7 @@ func (bc *Blockchain) AddBlock(block *block.Block) error {
// AddHeaders processes the given headers and add them to the
// HeaderHashList. It expects headers to be sorted by index.
func (bc *Blockchain) AddHeaders(headers ...*block.Header) error {
return bc.addHeaders(bc.config.Ledger.VerifyBlocks, headers...)
return bc.addHeaders(!bc.config.SkipBlockVerification, headers...)
}
// addHeaders is an internal implementation of AddHeaders (`verify` parameter

View file

@ -421,7 +421,7 @@ func TestBlockchain_AddBadBlock(t *testing.T) {
e.SignBlock(b)
check(t, b, nil)
check(t, b, func(c *config.Blockchain) {
c.Ledger.VerifyBlocks = false
c.SkipBlockVerification = true
})
b = e.NewUnsignedBlock(t)
@ -429,7 +429,7 @@ func TestBlockchain_AddBadBlock(t *testing.T) {
e.SignBlock(b)
check(t, b, nil)
check(t, b, func(c *config.Blockchain) {
c.Ledger.VerifyBlocks = false
c.SkipBlockVerification = true
})
tx = e.NewUnsignedTx(t, neoHash, "transfer", acc.ScriptHash(), util.Uint160{1, 2, 3}, 1, nil) // Check the good tx.
@ -438,7 +438,7 @@ func TestBlockchain_AddBadBlock(t *testing.T) {
e.SignBlock(b)
check(t, b, func(c *config.Blockchain) {
c.VerifyTransactions = true
c.Ledger.VerifyBlocks = true
c.SkipBlockVerification = false
})
}

View file

@ -323,7 +323,7 @@ func (s *Module) AddBlock(block *block.Block) error {
if s.bc.GetConfig().StateRootInHeader != block.StateRootEnabled {
return fmt.Errorf("stateroot setting mismatch: %v != %v", s.bc.GetConfig().StateRootInHeader, block.StateRootEnabled)
}
if s.bc.GetConfig().Ledger.VerifyBlocks {
if !s.bc.GetConfig().SkipBlockVerification {
merkle := block.ComputeMerkleRoot()
if !block.MerkleRoot.Equals(merkle) {
return errors.New("invalid block: MerkleRoot mismatch")

View file

@ -147,9 +147,6 @@ func NewSingleWithCustomConfigAndStore(t testing.TB, f func(cfg *config.Blockcha
ValidatorsCount: 1,
VerifyTransactions: true,
},
Ledger: config.Ledger{
VerifyBlocks: true,
},
}
if f != nil {
@ -208,9 +205,6 @@ func NewMultiWithCustomConfigAndStoreNoCheck(t testing.TB, f func(*config.Blockc
ValidatorsCount: 4,
VerifyTransactions: true,
},
Ledger: config.Ledger{
VerifyBlocks: true,
},
}
if f != nil {
f(&cfg)

View file

@ -132,7 +132,7 @@ func newChain() (*core.Blockchain, error) {
if err != nil {
return nil, err
}
unitTestNetCfg.ApplicationConfiguration.VerifyBlocks = false
unitTestNetCfg.ApplicationConfiguration.SkipBlockVerification = true
zapCfg := zap.NewDevelopmentConfig()
zapCfg.Level = zap.NewAtomicLevelAt(zapcore.InfoLevel)
log, err := zapCfg.Build()