diff --git a/nimbus/p2p/clique.nim b/nimbus/p2p/clique.nim index a168c02b2..80dfa98d0 100644 --- a/nimbus/p2p/clique.nim +++ b/nimbus/p2p/clique.nim @@ -214,7 +214,7 @@ proc verifyCascadingFields(c: var Clique; header: BlockHeader; if (header.blockNumber mod c.cfg.epoch.u256) == 0: let signersList = snap.value.signers - extraList = header.extraData.extraDataSigners + extraList = header.extraData.extraDataAddresses if signersList != extraList: return err((errMismatchingCheckpointSigners,"")) diff --git a/nimbus/p2p/clique/clique_utils.nim b/nimbus/p2p/clique/clique_utils.nim index 4dfba0ecb..1fadcc8d0 100644 --- a/nimbus/p2p/clique/clique_utils.nim +++ b/nimbus/p2p/clique/clique_utils.nim @@ -89,7 +89,7 @@ proc cliqueResultErr*(w: CliqueError): CliqueResult = err(w) -proc extraDataSigners*(extraData: Blob): seq[EthAddress] = +proc extraDataAddresses*(extraData: Blob): seq[EthAddress] = ## Extract signer addresses from extraData header field proc toEthAddress(a: openArray[byte]; start: int): EthAddress {.inline.} = diff --git a/nimbus/p2p/clique/recent_snaps.nim b/nimbus/p2p/clique/recent_snaps.nim index 24304c3eb..29eb73de5 100644 --- a/nimbus/p2p/clique/recent_snaps.nim +++ b/nimbus/p2p/clique/recent_snaps.nim @@ -76,44 +76,51 @@ proc say(d: RecentDesc; v: varargs[string,`$`]) = if d.debug: stderr.write "*** " & v.join & "\n" +proc say(rs: var RecentSnaps; v: varargs[string,`$`]) = + ## Debugging output + ppExceptionWrap: + if rs.debug: + stderr.write "*** " & v.join & "\n" + proc getPrettyPrinters(d: RecentDesc): var PrettyPrinters = ## Mixin for pretty printers, see `clique/clique_cfg.pp()` d.cfg.prettyPrint -# clique/clique.go(394): if number == 0 || (number%c.config.Epoch [..] -proc canDiskCheckPointOk(d: RecentDesc): bool {. - gcsafe, raises: [Defect,RlpError].} = - # If we're at the genesis, snapshot the initial state. + +proc canDiskCheckPointOk(d: RecentDesc): + bool {.inline, raises: [Defect,RlpError].} = + + # clique/clique.go(394): if number == 0 || (number%c.config.Epoch [..] if d.args.blockNumber.isZero: + # If we're at the genesis, snapshot the initial state. return true - # Alternatively if we're at a checkpoint block without a parent - # (light client CHT), or we have piled up more headers than allowed - # to be re-orged (chain reinit from a freezer), consider the - # checkpoint trusted and snapshot it. + if (d.args.blockNumber mod d.cfg.epoch) == 0: + # Alternatively if we're at a checkpoint block without a parent + # (light client CHT), or we have piled up more headers than allowed + # to be re-orged (chain reinit from a freezer), consider the + # checkpoint trusted and snapshot it. if FULL_IMMUTABILITY_THRESHOLD < d.local.headers.len: return true if d.cfg.dbChain.getBlockHeaderResult(d.args.blockNumber - 1).isErr: return true -proc isCheckPointOk(number: BlockNumber): bool = - (number mod CHECKPOINT_INTERVAL) == 0 - # ------------------------------------------------------------------------------ # Private functions # ------------------------------------------------------------------------------ -# clique/clique.go(383): if number%checkpointInterval == 0 [..] -proc tryDiskSnapshot(d: RecentDesc; snap: var Snapshot): bool = - if d.args.blockNumber.isCheckPointOk: +proc tryLoadDiskSnapshot(d: RecentDesc; snap: var Snapshot): bool {.inline.} = + # clique/clique.go(383): if number%checkpointInterval == 0 [..] + if (d.args.blockNumber mod CHECKPOINT_INTERVAL) == 0: if snap.loadSnapshot(d.cfg, d.args.blockHash).isOk: trace "Loaded voting snapshot from disk", blockNumber = d.args.blockNumber, blockHash = d.args.blockHash return true -proc tryDiskCheckPoint(d: RecentDesc; snap: var Snapshot): bool {. - gcsafe, raises: [Defect,RlpError].} = + +proc tryStoreDiskCheckPoint(d: RecentDesc; snap: var Snapshot): + bool {.gcsafe, raises: [Defect,RlpError].} = if d.canDiskCheckPointOk: # clique/clique.go(395): checkpoint := chain.GetHeaderByNumber [..] let checkPoint = d.cfg.dbChain.getBlockHeaderResult(d.args.blockNumber) @@ -121,8 +128,8 @@ proc tryDiskCheckPoint(d: RecentDesc; snap: var Snapshot): bool {. return false let hash = checkPoint.value.hash - signersList = checkPoint.value.extraData.extraDataSigners - snap.initSnapshot(d.cfg, d.args.blockNumber, hash, signersList) + accountList = checkPoint.value.extraData.extraDataAddresses + snap.initSnapshot(d.cfg, d.args.blockNumber, hash, accountList) snap.setDebug(d.debug) if snap.storeSnapshot.isOk: @@ -148,12 +155,12 @@ proc initRecentSnaps*(rs: var RecentSnaps; while true: # If an on-disk checkpoint snapshot can be found, use that - if d.tryDiskSnapshot(snap): + if d.tryLoadDiskSnapshot(snap): # clique/clique.go(386): snap = s break # Save checkpoint e.g. when at the genesis .. - if d.tryDiskCheckPoint(snap): + if d.tryStoreDiskCheckPoint(snap): # clique/clique.go(407): log.Info("Stored [..] break @@ -164,7 +171,7 @@ proc initRecentSnaps*(rs: var RecentSnaps; header = d.args.parents[^1] # clique/clique.go(416): if header.Hash() != hash [..] - if header.hash != d.args.blockHash and + if header.hash != d.args.blockHash or header.blockNumber != d.args.blockNumber: return err((errUnknownAncestor,"")) d.args.parents.setLen(d.args.parents.len-1) @@ -189,15 +196,16 @@ proc initRecentSnaps*(rs: var RecentSnaps; block: # clique/clique.go(434): snap, err := snap.apply(headers) d.say "recentSnaps => applySnapshot([", - d.local.headers.mapIt("#" & $it.blockNumber.truncate(int)) - .join(",").string, "])" + d.local.headers.mapIt("#" & $it.blockNumber.truncate(int)) + .join(",").string, "])" let rc = snap.applySnapshot(d.local.headers) d.say "recentSnaps => applySnapshot() => ", rc.pp if rc.isErr: return err(rc.error) # If we've generated a new checkpoint snapshot, save to disk - if snap.blockNumber.isCheckPointOk and 0 < d.local.headers.len: + if (snap.blockNumber mod CHECKPOINT_INTERVAL) == 0 and + 0 < d.local.headers.len: var rc = snap.storeSnapshot if rc.isErr: return err(rc.error) @@ -222,6 +230,7 @@ proc setDebug*(rs: var RecentSnaps; debug: bool) = proc getRecentSnaps*(rs: var RecentSnaps; args: RecentArgs): auto {. gcsafe, raises: [Defect,CatchableError].} = ## Get snapshot from cache or disk + rs.say "getRecentSnap #", args.blockNumber.truncate(uint64) rs.cache.getLruItem: RecentDesc(cfg: rs.cfg, debug: rs.debug, diff --git a/nimbus/p2p/clique/snapshot.nim b/nimbus/p2p/clique/snapshot.nim index 4594ae6a8..c1424e856 100644 --- a/nimbus/p2p/clique/snapshot.nim +++ b/nimbus/p2p/clique/snapshot.nim @@ -30,6 +30,7 @@ import ./clique_cfg, ./clique_defs, ./clique_poll, + ./clique_utils, ./ec_recover, chronicles, eth/[common, rlp, trie/db] @@ -202,7 +203,7 @@ proc applySnapshot*(s: var Snapshot; headers: openArray[BlockHeader]): CliqueResult {. gcsafe, raises: [Defect,CatchableError].} = ## Initialises an authorization snapshot `snap` by applying the `headers` - ## to the argument snapshot `s`. + ## to the argument snapshot desciptor `s`. s.say "applySnapshot ", s.pp(headers).join("\n" & ' '.repeat(18)) @@ -238,7 +239,23 @@ proc applySnapshot*(s: var Snapshot; # Remove any votes on checkpoint blocks if (number mod s.cfg.epoch) == 0: - s.data.ballot.initCliquePoll + # clique/snapshot.go(210): snap.Votes = nil + s.say "applySnapshot epoch => reset, state=", s.pp(41) + + # This part differs from the go implementation in that the `signer` + # list is re-assigned. The original implementation silently assumes + # that the `signer` list is the same as the previous one but this is + # not enforced. + # + # The eip225 discussion has it as: [..] where every epoch transition + # flushes all pending votes. Furthermore, these epoch transitions can + # also act as stateless checkpoints containing the list of current + # authorized signers within the header extra-data. This permits clients + # to sync up based only on a checkpoint hash without having to replay + # all the voting that was done on the chain up to that point. It also + # allows the genesis header to fully define the chain, containing the + # list of initial signers. + s.data.ballot.initCliquePoll(header.extraData.extraDataAddresses) s.data.ballot.setDebug(s.data.debug) # Delete the oldest signer from the recent list to allow it signing again diff --git a/tests/test_clique.nim b/tests/test_clique.nim index d2ac2cd17..e507a9f9e 100644 --- a/tests/test_clique.nim +++ b/tests/test_clique.nim @@ -28,18 +28,19 @@ proc cliqueMain*(noisy = defined(debug)) = suite "Clique PoA Snapshot": var pool = newTesterPool() - skipSet = {20, 23, 24} - testSet = {0 .. 99} + skipSet = {999} + testSet = {0 .. 999} pool.setDebug(noisy) # clique/snapshot_test.go(379): for i, tt := range tests { for tt in voterSamples.filterIt(it.id in testSet): - pool.say "\n" - test &"Snapshots {tt.id}: {tt.info.substr(0,50)}...": + + test &"Snapshots {tt.id: 2}: {tt.info.substr(0,50)}...": + pool.say "\n" if tt.id in skipSet: - echo &"Test {tt.id} skipped" + skip() else: # Assemble a chain of headers from the cast votes