Skip to content
Open
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@

### Fixed

- [#7412](https://github.com/ChainSafe/forest/issues/7412): Block validation on quicknet now rejects headers whose beacon entries do not cover every epoch since the parent tipset
Comment thread
EclesioMeloJunior marked this conversation as resolved.
Outdated

## Forest v0.35.0 "Shravan"

Non-mandatory release for all node operators. It includes some fixes and improvements, notably around state-related RPC. Note that this release contains breaking changes, so please read the changelog carefully before upgrading.
Expand Down
6 changes: 4 additions & 2 deletions src/beacon/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ pub use drand::*;
#[cfg(test)]
pub mod mock_beacon;
#[cfg(test)]
mod tests {
mod drand;
pub mod tests {
// `pub` so that helpers such as `drand::new_beacon_quicknet` can be shared with
// tests in other modules.
pub mod drand;
}
2 changes: 1 addition & 1 deletion src/beacon/tests/drand.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ fn new_beacon_mainnet() -> DrandBeacon {
)
}

fn new_beacon_quicknet() -> DrandBeacon {
pub fn new_beacon_quicknet() -> DrandBeacon {
DrandBeacon::new(
1598306400,
30,
Expand Down
85 changes: 84 additions & 1 deletion src/blocks/header.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,48 @@ impl RawBlockHeader {
));
}

// An unchained beacon carries no link between consecutive entries, so nothing
Comment thread
EclesioMeloJunior marked this conversation as resolved.
Outdated
// but these checks ties them to the epochs they are supposed to cover. A block
// covers every epoch since its parent - normally just its own, but null rounds
// can happen and it must carry exactly one entry per covered epoch, in
// ascending order.
if curr_beacon.network().is_unchained() {
Comment thread
EclesioMeloJunior marked this conversation as resolved.
let covered_epochs = self.epoch - parent_epoch;
let found_entries = i64::try_from(self.beacon_entries.len())
.map_err(|_| Error::Validation("too many beacon entries".into()))?;
if found_entries != covered_epochs {
return Err(Error::Validation(
format!(
"expected {covered_epochs} beacon entries for epochs {}..={}, got: {found_entries}",
parent_epoch + 1,
self.epoch,
)
.into(),
));
}
Comment thread
EclesioMeloJunior marked this conversation as resolved.
Outdated

// Lengths match, so `zip` pairs every entry with the epoch it must cover.
for (beacon_entry, lookup_epoch) in self
.beacon_entries
.iter()
.zip((parent_epoch + 1)..=self.epoch)
Comment thread
EclesioMeloJunior marked this conversation as resolved.
Outdated
{
let expected_round =
curr_beacon.max_beacon_round_for_epoch(network_version, lookup_epoch);
if beacon_entry.round() != expected_round {
return Err(Error::Validation(
format!(
"expected max round for epoch {} to be {}, got: {}",
lookup_epoch,
expected_round,
beacon_entry.round(),
)
.into(),
));
}
}
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if !curr_beacon
.verify_entries(&self.beacon_entries, prev_entry)
.map_err(|e| Error::Validation(format!("{e:#}").into()))?
Expand Down Expand Up @@ -352,7 +394,10 @@ impl<'de> Deserialize<'de> for CachingBlockHeader {
#[cfg(test)]
mod tests {
use super::*;
use crate::beacon::{BeaconEntry, BeaconPoint, BeaconSchedule, mock_beacon::MockBeacon};
use crate::beacon::{
BeaconEntry, BeaconPoint, BeaconSchedule, mock_beacon::MockBeacon,
tests::drand::new_beacon_quicknet,
};
use crate::blocks::{CachingBlockHeader, Error};
use crate::shim::clock::ChainEpoch;
use crate::shim::{address::Address, version::NetworkVersion};
Expand Down Expand Up @@ -432,4 +477,42 @@ mod tests {
*FILECOIN_GENESIS_CID
);
}

#[tokio::test]
Comment thread
EclesioMeloJunior marked this conversation as resolved.
async fn validate_block_drand_accepts_correct_entries_quicknet() {
// (parent epoch, its beacon round, block epoch, rounds the header must carry)
let cases = [
// no gaps, expected only a single entry
(6216199, 30662992, 6216200, vec![30663002]),
// one gap (6216199) so the block 6216200 should have its and the 6216199 beacon entries
(6216198, 30662982, 6216200, vec![30662992, 30663002]),
];

let schedule = BeaconSchedule(vec![BeaconPoint::new(0, new_beacon_quicknet())]);

for (parent_epoch, prev_round, epoch, rounds) in cases {
let (_, curr_beacon) = schedule.beacon_for_epoch(epoch).unwrap();

// fetch the real entries so BLS verifies correctly
let mut beacon_entries = Vec::with_capacity(rounds.len());
for round in rounds {
beacon_entries.push(curr_beacon.entry(round).await.unwrap());
}

// Unchained verification never reads `prev_entry`, only its round is used
// to decide whether drand has ticked since the parent.
let prev_entry = BeaconEntry::new(prev_round, vec![]);

let header = RawBlockHeader {
miner_address: Address::new_id(0),
epoch,
beacon_entries,
..Default::default()
};

header
.validate_block_drand(NetworkVersion::V22, &schedule, parent_epoch, &prev_entry)
.unwrap_or_else(|e| panic!("epoch {epoch}, parent {parent_epoch}: {e}"));
}
}
}
Loading