From 165692fdd644dfb5d09400ea2cacd3c9f85b4d53 Mon Sep 17 00:00:00 2001 From: Dave Collins Date: Fri, 27 Nov 2020 09:25:49 -0600 Subject: [PATCH] blockchain: Make CheckBlockSanity context free. This removes the now unused checkBlockSanityContextual since all the checks that rely on context are now in the proper context-based function and restores the context-free nature of checkBlockSanity and its exported variant. This is part of an overall effort to ultimately move all checks that require full contextual information into the proper location since, as the comments call out, sanity functions are supposed to be entirely context free and positional functions are not supposed to have any checks that rely on having the full block data of all ancestors available. These restrictions are important for planned future work. --- blockchain/process.go | 19 +----------------- blockchain/validate.go | 39 ++++++------------------------------- blockchain/validate_test.go | 2 +- rpcadaptors.go | 9 +-------- 4 files changed, 9 insertions(+), 60 deletions(-) diff --git a/blockchain/process.go b/blockchain/process.go index 306efa77..e1f54a98 100644 --- a/blockchain/process.go +++ b/blockchain/process.go @@ -66,7 +66,7 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (in } // Perform preliminary sanity checks on the block and its transactions. - err := checkBlockSanityContextFree(block, b.timeSource, flags, b.chainParams) + err := checkBlockSanity(block, b.timeSource, flags, b.chainParams) if err != nil { return 0, err } @@ -81,23 +81,6 @@ func (b *BlockChain) ProcessBlock(block *dcrutil.Block, flags BehaviorFlags) (in return 0, ruleError(ErrMissingParent, str) } - // Perform preliminary sanity checks on the block and its transactions that - // depend on the state of the treasury agenda. Note that these checks - // really ultimately need to be done later in the context-dependent block - // checking, however, they are done here for now as a stop gap to ensure - // they are not applied to orphan blocks from further in the chain which may - // have the new rules active before the local chain is far enough along for - // them to be active. - isTreasuryEnabled, err := b.isTreasuryAgendaActiveByHash(prevHash) - if err != nil { - return 0, err - } - err = checkBlockSanityContextual(block, b.timeSource, flags, b.chainParams, - isTreasuryEnabled) - if err != nil { - return 0, err - } - // The block has passed all context independent checks and appears sane // enough to potentially accept it into the block chain. forkLen, err := b.maybeAcceptBlock(block, flags) diff --git a/blockchain/validate.go b/blockchain/validate.go index ec2a70e1..1b3f2184 100644 --- a/blockchain/validate.go +++ b/blockchain/validate.go @@ -668,31 +668,12 @@ func checkBlockHeaderSanity(header *wire.BlockHeader, timeSource MedianTimeSourc } // checkBlockSanity performs some preliminary checks on a block to ensure it is -// sane before continuing with block processing. These checks are supposed to -// be context free but are not due to the treasury agenda. +// sane before continuing with block processing. These checks are context +// free. // // The flags do not modify the behavior of this function directly, however they // are needed to pass along to checkBlockHeaderSanity. -// -// Note that this is a giant hack to separate the original context free -// function into two functions due to the treasury agenda induced contextual -// isTreasuryEnabled flag. The contextual checks have been pulled out in order -// to maintain order and assumptions without having to discard the -// isTreasuryEnabled checks from the function. -func checkBlockSanity(block *dcrutil.Block, timeSource MedianTimeSource, flags BehaviorFlags, chainParams *chaincfg.Params, isTreasuryEnabled bool) error { - err := checkBlockSanityContextFree(block, timeSource, flags, - chainParams) - if err != nil { - return err - } - return checkBlockSanityContextual(block, timeSource, flags, - chainParams, isTreasuryEnabled) -} - -// checkBlockSanityContextFree performs some preliminary checks on a block to -// ensure it is sane before continuing with block processing. These checks are -// context free. -func checkBlockSanityContextFree(block *dcrutil.Block, timeSource MedianTimeSource, flags BehaviorFlags, chainParams *chaincfg.Params) error { +func checkBlockSanity(block *dcrutil.Block, timeSource MedianTimeSource, flags BehaviorFlags, chainParams *chaincfg.Params) error { msgBlock := block.MsgBlock() header := &msgBlock.Header err := checkBlockHeaderSanity(header, timeSource, flags, chainParams) @@ -813,18 +794,11 @@ func checkBlockSanityContextFree(block *dcrutil.Block, timeSource MedianTimeSour return nil } -// checkBlockSanityContextual performs some preliminary checks on a block to -// ensure it is sane before continuing with block processing. These checks are -// contextual. -func checkBlockSanityContextual(block *dcrutil.Block, timeSource MedianTimeSource, flags BehaviorFlags, chainParams *chaincfg.Params, isTreasuryEnabled bool) error { - return nil -} - // CheckBlockSanity performs some preliminary checks on a block to ensure it is // sane before continuing with block processing. These checks are context // free. -func CheckBlockSanity(block *dcrutil.Block, timeSource MedianTimeSource, chainParams *chaincfg.Params, isTreasuryEnabled bool) error { - return checkBlockSanity(block, timeSource, BFNone, chainParams, isTreasuryEnabled) +func CheckBlockSanity(block *dcrutil.Block, timeSource MedianTimeSource, chainParams *chaincfg.Params) error { + return checkBlockSanity(block, timeSource, BFNone, chainParams) } // checkBlockHeaderPositional performs several validation checks on the block @@ -3795,8 +3769,7 @@ func (b *BlockChain) CheckConnectBlockTemplate(block *dcrutil.Block) error { if err != nil { return err } - err = checkBlockSanity(block, b.timeSource, flags, b.chainParams, - isTreasuryEnabled) + err = checkBlockSanity(block, b.timeSource, flags, b.chainParams) if err != nil { return err } diff --git a/blockchain/validate_test.go b/blockchain/validate_test.go index 8be6ccc4..1378b78c 100644 --- a/blockchain/validate_test.go +++ b/blockchain/validate_test.go @@ -532,7 +532,7 @@ func TestCheckBlockSanity(t *testing.T) { params := chaincfg.RegNetParams() timeSource := NewMedianTime() block := dcrutil.NewBlock(&badBlock) - err := CheckBlockSanity(block, timeSource, params, noTreasury) + err := CheckBlockSanity(block, timeSource, params) if err == nil { t.Fatalf("block should fail.\n") } diff --git a/rpcadaptors.go b/rpcadaptors.go index b381fd0d..9f60a783 100644 --- a/rpcadaptors.go +++ b/rpcadaptors.go @@ -483,14 +483,7 @@ var _ rpcserver.SanityChecker = (*rpcSanityChecker)(nil) // // This function is part of the rpcserver.SanityChecker interface implementation. func (s *rpcSanityChecker) CheckBlockSanity(block *dcrutil.Block) error { - pHash := &block.MsgBlock().Header.PrevBlock - isTreasuryEnabled, err := s.chain.IsTreasuryAgendaActive(pHash) - if err != nil { - return err - } - - return blockchain.CheckBlockSanity(block, s.timeSource, s.chainParams, - isTreasuryEnabled) + return blockchain.CheckBlockSanity(block, s.timeSource, s.chainParams) } // rpcBlockTemplater provides a block template generator for use with the