Skip to content

Commit 87157e0

Browse files
authored
Block command-line supplied invalid blocks earlier (#7714)
* Block command-line supplied invalid blocks earlier For some reason, they're only blocked in the CL fallback of EL processing, which means it doesn't work with a responding EL. Also add initial smoke tests for non-bellatrix block processing. * Apply suggestion from @arnetheduck
1 parent 4aab8d5 commit 87157e0

File tree

5 files changed

+260
-147
lines changed

5 files changed

+260
-147
lines changed

AllTests-mainnet.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ AllTests-mainnet
135135
## Block processor [Preset: mainnet]
136136
```diff
137137
+ Invalidate block root [Preset: mainnet] OK
138+
+ Process a block from each fork (without blobs) [Preset: mainnet] OK
138139
+ Reverse order block add & get [Preset: mainnet] OK
139140
```
140141
## Block quarantine

beacon_chain/gossip_processing/block_processor.nim

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -499,9 +499,6 @@ proc verifyPayload(
499499
# There are no `blob_kzg_commitments` before Deneb to compare against
500500
discard
501501

502-
if signedBlock.root in self.invalidBlockRoots:
503-
returnWithError "Block root treated as invalid via config", $signedBlock.root
504-
505502
ok OptimisticStatus.notValidated
506503
else:
507504
ok OptimisticStatus.valid
@@ -580,6 +577,12 @@ proc storeBlock(
580577
chronos.nanoseconds((slotTime - wallTime).nanoseconds)
581578
deadline = sleepAsync(deadlineTime)
582579

580+
if signedBlock.root in self.invalidBlockRoots:
581+
warn "Block root treated as invalid via config",
582+
blck = shortLog(signedBlock.message),
583+
blockRoot = shortLog(signedBlock.root)
584+
return err(VerifierError.Invalid)
585+
583586
# We have to be careful that there exists only one in-flight entry point
584587
# for adding blocks or the checks performed in `checkHeadBlock` might
585588
# be invalidated (ie a block could be added while we wait for EL response

beacon_chain/spec/helpers.nim

Lines changed: 29 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -535,19 +535,36 @@ func toExecutionBlockHeader(
535535
requestsHash : requestsHash) # EIP-7685
536536

537537
func compute_execution_block_hash*(
538-
body: ForkyBeaconBlockBody,
539-
parentRoot: Eth2Digest): Eth2Digest =
540-
when typeof(body).kind >= ConsensusFork.Electra:
541-
body.execution_payload.toExecutionBlockHeader(
542-
Opt.some parentRoot, Opt.some body.execution_requests.computeRequestsHash())
543-
.computeRlpHash().to(Eth2Digest)
544-
elif typeof(body).kind >= ConsensusFork.Deneb:
545-
body.execution_payload.toExecutionBlockHeader(
546-
Opt.some parentRoot)
547-
.computeRlpHash().to(Eth2Digest)
538+
consensusFork: static ConsensusFork,
539+
payload: ForkyExecutionPayload,
540+
parentRoot: Eth2Digest,
541+
requestsHash = Opt.none(EthHash32),
542+
): Eth2Digest =
543+
let header =
544+
when consensusFork >= ConsensusFork.Electra:
545+
payload.toExecutionBlockHeader(Opt.some parentRoot, requestsHash)
546+
elif consensusFork >= ConsensusFork.Deneb:
547+
payload.toExecutionBlockHeader(Opt.some parentRoot)
548+
else:
549+
payload.toExecutionBlockHeader(Opt.none(Eth2Digest))
550+
551+
header.computeRlpHash().to(Eth2Digest)
552+
553+
func compute_execution_block_hash*(
554+
body: ForkyBeaconBlockBody, parentRoot: Eth2Digest
555+
): Eth2Digest =
556+
const consensusFork = typeof(body).kind
557+
when consensusFork >= ConsensusFork.Electra:
558+
compute_execution_block_hash(
559+
consensusFork,
560+
body.execution_payload,
561+
parentRoot,
562+
Opt.some body.execution_requests.computeRequestsHash(),
563+
)
548564
else:
549-
body.execution_payload.toExecutionBlockHeader(Opt.none(Eth2Digest))
550-
.computeRlpHash().to(Eth2Digest)
565+
compute_execution_block_hash(
566+
consensusFork, body.execution_payload, parentRoot
567+
)
551568

552569
func compute_execution_block_hash*(blck: ForkyBeaconBlock): Eth2Digest =
553570
blck.body.compute_execution_block_hash(blck.parent_root)

tests/test_block_processor.nim

Lines changed: 65 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,10 @@ import
1818
../beacon_chain/gossip_processing/block_processor,
1919
../beacon_chain/consensus_object_pools/[
2020
attestation_pool, blockchain_dag, blob_quarantine, block_quarantine,
21-
block_clearance, consensus_manager],
21+
block_clearance, consensus_manager,
22+
],
2223
../beacon_chain/el/el_manager,
23-
./testutil, ./testdbutil, ./testblockutil
24+
./[testblockutil, testdbutil, testutil]
2425

2526
from chronos/unittest2/asynctests import asyncTest
2627
from ../beacon_chain/spec/eth2_apis/dynamic_fee_recipients import
@@ -40,43 +41,57 @@ suite "Block processor" & preset():
4041
var res = defaultRuntimeConfig
4142
res.ALTAIR_FORK_EPOCH = GENESIS_EPOCH
4243
res.BELLATRIX_FORK_EPOCH = GENESIS_EPOCH
44+
res.CAPELLA_FORK_EPOCH = Epoch(1)
45+
res.DENEB_FORK_EPOCH = Epoch(2)
46+
res.ELECTRA_FORK_EPOCH = Epoch(3)
47+
res.FULU_FORK_EPOCH = Epoch(4)
4348
res
4449
db = cfg.makeTestDB(SLOTS_PER_EPOCH)
4550
validatorMonitor = newClone(ValidatorMonitor.init(cfg.timeParams))
4651
dag = init(ChainDAGRef, cfg, db, validatorMonitor, {})
47-
var
4852
taskpool = Taskpool.new()
4953
quarantine = newClone(Quarantine.init(cfg))
5054
blobQuarantine = newClone(BlobQuarantine())
5155
dataColumnQuarantine = newClone(ColumnQuarantine())
5256
attestationPool = newClone(AttestationPool.init(dag, quarantine))
5357
elManager = new ELManager # TODO: initialise this properly
54-
actionTracker: ActionTracker
58+
actionTracker = default(ActionTracker)
5559
consensusManager = ConsensusManager.new(
56-
dag, attestationPool, quarantine, elManager, actionTracker,
57-
newClone(DynamicFeeRecipientsStore.init()), "",
58-
Opt.some default(Eth1Address), defaultGasLimit)
60+
dag,
61+
attestationPool,
62+
quarantine,
63+
elManager,
64+
actionTracker,
65+
newClone(DynamicFeeRecipientsStore.init()),
66+
"",
67+
Opt.some default(Eth1Address),
68+
defaultGasLimit,
69+
)
5970
state = newClone(dag.headState)
60-
cache: StateCache
61-
info: ForkedEpochInfo
62-
cfg.process_slots(
63-
state[], cfg.lastPremergeSlotInTestCfg, cache, info, {}).expect("OK")
64-
var
65-
b1 = addTestBlock(state[], cache, cfg = cfg).bellatrixData
66-
b2 = addTestBlock(state[], cache, cfg = cfg).bellatrixData
6771
getTimeFn = proc(): BeaconTime =
68-
b2.message.slot.start_beacon_time(cfg.timeParams)
72+
getStateField(state[], slot).start_beacon_time(cfg.timeParams)
6973
batchVerifier = BatchVerifier.new(rng, taskpool)
70-
processor = BlockProcessor.new(
71-
false, "", "", batchVerifier, consensusManager,
72-
validatorMonitor, blobQuarantine, dataColumnQuarantine, getTimeFn)
74+
var
75+
cache: StateCache
76+
info: ForkedEpochInfo
77+
78+
cfg.process_slots(state[], cfg.lastPremergeSlotInTestCfg, cache, info, {}).expect(
79+
"OK"
80+
)
7381

7482
asyncTest "Reverse order block add & get" & preset():
75-
let missing = await processor.addBlock(MsgSource.gossip, b2, noSidecars)
83+
let
84+
processor = BlockProcessor.new(
85+
false, "", "", batchVerifier, consensusManager, validatorMonitor,
86+
blobQuarantine, dataColumnQuarantine, getTimeFn,
87+
)
88+
b1 = addTestBlock(state[], cache, cfg = cfg).bellatrixData
89+
b2 = addTestBlock(state[], cache, cfg = cfg).bellatrixData
7690

77-
check: missing.error == VerifierError.MissingParent
91+
missing = await processor.addBlock(MsgSource.gossip, b2, noSidecars)
7892

7993
check:
94+
missing.error == VerifierError.MissingParent
8095
not dag.containsForkBlock(b2.root) # Unresolved, shouldn't show up
8196

8297
FetchRecord(root: b1.root) in quarantine[].checkMissing(32)
@@ -94,8 +109,7 @@ suite "Block processor" & preset():
94109
while processor[].hasBlocks():
95110
poll()
96111

97-
let
98-
b2Get = dag.getBlockRef(b2.root)
112+
let b2Get = dag.getBlockRef(b2.root)
99113

100114
check:
101115
b2Get.isSome()
@@ -126,6 +140,8 @@ suite "Block processor" & preset():
126140

127141
asyncTest "Invalidate block root" & preset():
128142
let
143+
b1 = addTestBlock(state[], cache, cfg = cfg).bellatrixData
144+
b2 = addTestBlock(state[], cache, cfg = cfg).bellatrixData
129145
processor = BlockProcessor.new(
130146
false, "", "", batchVerifier, consensusManager,
131147
validatorMonitor, blobQuarantine, dataColumnQuarantine,
@@ -156,3 +172,30 @@ suite "Block processor" & preset():
156172
res == Result[void, VerifierError].err VerifierError.Invalid
157173
dag.containsForkBlock(b1.root)
158174
not dag.containsForkBlock(b2.root)
175+
176+
asyncTest "Process a block from each fork (without blobs)" & preset():
177+
let processor = BlockProcessor.new(
178+
false, "", "", batchVerifier, consensusManager, validatorMonitor, blobQuarantine,
179+
dataColumnQuarantine, getTimeFn,
180+
)
181+
182+
debugGloasComment "TODO testing"
183+
for consensusFork in ConsensusFork.Bellatrix .. ConsensusFork.Fulu:
184+
process_slots(
185+
cfg,
186+
state[],
187+
max(
188+
getStateField(state[], slot) + 1,
189+
cfg.consensusForkEpoch(consensusFork).start_slot,
190+
),
191+
cache,
192+
info,
193+
{},
194+
)
195+
.expect("OK")
196+
197+
withState(state[]):
198+
let b0 = addTestEngineBlock(cfg, consensusFork, forkyState, cache)
199+
discard await processor.addBlock(
200+
MsgSource.gossip, b0.blck, b0.blobsBundle.toSidecarsOpt(consensusFork)
201+
)

0 commit comments

Comments
 (0)