Skip to content

Commit 85826e6

Browse files
committed
Trace invalid blocks in unviable quarantine
A block may become unviable for two main reasons: * the fork it was on is orphaned by chain finality - ie the block was valid in some history, but not any more in the canonical one * the block is invalid, either because it is itself invalid or it builds on an invalid block Until now we've treated both kinds the same when it comes to the `unviable` quarantine - because of this, when checking gossip rules the client has in some cases opted for the more lenient "IGNORE" respons instead of using "REJECT". This PR brings the client more closely in line with ignore/reject rules by tracking the original error that led to the unviablility. * use `minilru` instead of `OrderedTable` for simplified capacity management and performance * simplify quarantine operations that deal with unviables * remove unused quarantine events * keep track of the block being processed to avoid it being readded to the quarantine during processing
1 parent 80ade8d commit 85826e6

File tree

10 files changed

+406
-341
lines changed

10 files changed

+406
-341
lines changed

beacon_chain.nimble

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ license = "MIT or Apache License 2.0"
1414

1515
requires(
1616
"nim == 2.2.4",
17-
"https://github.com/status-im/NimYAML",
1817
"bearssl",
1918
"blscurve",
2019
"chronicles",
@@ -28,6 +27,7 @@ requires(
2827
"libbacktrace",
2928
"libp2p",
3029
"metrics",
30+
"minilru",
3131
"nat_traversal",
3232
"nimcrypto",
3333
"normalize",
@@ -41,11 +41,12 @@ requires(
4141
"stint",
4242
"taskpools",
4343
"testutils",
44+
"toml_serialization",
4445
"unicodedb >= 0.10",
4546
"unittest2",
47+
"yaml",
4648
"web3",
4749
"zlib",
48-
"toml_serialization",
4950
"https://github.com/status-im/nim-kzg4844.git",
5051
"zxcvbn"
5152
)

beacon_chain/consensus_object_pools/attestation_pool.nim

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -953,19 +953,23 @@ proc getBeaconHead*(
953953
proc selectOptimisticHead*(
954954
pool: var AttestationPool, wallTime: BeaconTime): Opt[BeaconHead] =
955955
## Trigger fork choice and returns the new head block.
956-
let newHeadRoot = pool.forkChoice.get_head(pool.dag, wallTime)
957-
if newHeadRoot.isErr:
958-
error "Couldn't select head", err = newHeadRoot.error
959-
return err()
956+
let newHeadRoot = pool.forkChoice.get_head(pool.dag, wallTime).valueOr:
957+
error "Couldn't select head", err = error
958+
return Opt.none(BeaconHead)
960959

961-
let headBlock = pool.dag.getBlockRef(newHeadRoot.get()).valueOr:
960+
let headBlock = pool.dag.getBlockRef(newHeadRoot).valueOr:
962961
# This should normally not happen, but if the chain dag and fork choice
963962
# get out of sync, we'll need to try to download the selected head - in
964963
# the meantime, return nil to indicate that no new head was chosen
965-
warn "Fork choice selected unknown head, trying to sync",
966-
root = newHeadRoot.get()
967-
pool.quarantine[].addMissing(newHeadRoot.get())
968-
return err()
964+
965+
pool.quarantine[].addMissing(newHeadRoot).isOkOr:
966+
# The newly selected head is unviable for some reason - the only way out
967+
# here is that fork choice gets information about some other head
968+
warn "Fork choice selected unviable head - cannot sync", newHeadRoot, err = error
969+
return Opt.none(BeaconHead)
970+
971+
warn "Fork choice selected unknown head, trying to sync", newHeadRoot
972+
return Opt.none(BeaconHead)
969973

970974
ok pool.getBeaconHead(headBlock)
971975

0 commit comments

Comments
 (0)