Skip to content

Commit

Permalink
Adjust computation of block number to sign
Browse files Browse the repository at this point in the history
Substacting `1` to the result to account for the fact that a block range
end is exclusive.
  • Loading branch information
Alenar committed Jul 3, 2024
1 parent b7ccf42 commit afef448
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 15 deletions.
6 changes: 3 additions & 3 deletions mithril-aggregator/tests/create_certificate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,7 +142,7 @@ async fn create_certificate() {

comment!(
"Increase cardano chain block number to 185,
the state machine should be signing CardanoTransactions for block 180"
the state machine should be signing CardanoTransactions for block 179"
);
tester.increase_block_number(85, 185).await.unwrap();
cycle!(tester, "signing");
Expand All @@ -166,7 +166,7 @@ async fn create_certificate() {
.map(|s| s.signer_with_stake.clone().into())
.collect::<Vec<_>>(),
fixture.compute_and_encode_avk(),
SignedEntityType::CardanoTransactions(Epoch(1), 180),
SignedEntityType::CardanoTransactions(Epoch(1), 179),
ExpectedCertificate::genesis_identifier(&CardanoDbBeacon::new(
"devnet".to_string(),
1,
Expand Down Expand Up @@ -201,7 +201,7 @@ async fn create_certificate() {
.map(|s| s.signer_with_stake.clone().into())
.collect::<Vec<_>>(),
fixture.compute_and_encode_avk(),
SignedEntityType::CardanoTransactions(Epoch(1), 120),
SignedEntityType::CardanoTransactions(Epoch(1), 119),
ExpectedCertificate::genesis_identifier(&CardanoDbBeacon::new(
"devnet".to_string(),
1,
Expand Down
29 changes: 17 additions & 12 deletions mithril-common/src/entities/signed_entity_config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -131,20 +131,25 @@ impl CardanoTransactionsSigningConfig {
///
/// The formula is as follows:
///
/// `block_number = ⌊(tip.block_number - security_parameter) / step⌋ × step`
/// `block_number = ⌊(tip.block_number - security_parameter) / step⌋ × step - 1`
///
/// where `⌊x⌋` is the floor function which rounds to the greatest integer less than or equal to `x`.
///
/// *Note: The step is adjusted to be a multiple of the block range length in order
/// *Notes:*
/// * *The step is adjusted to be a multiple of the block range length in order
/// to guarantee that the block number signed in a certificate is effectively signed.*
/// * *1 is subtracted to the result because block range end is exclusive (ie: a BlockRange over
/// `30..45` finish at 44 included, 45 is included in the next block range).*
pub fn compute_block_number_to_be_signed(&self, block_number: BlockNumber) -> BlockNumber {
// TODO: See if we can remove this adjustment by including a "partial" block range in
// the signed data.
let adjusted_step = BlockRange::from_block_number(self.step).start;
// We can't have a step lower than the block range length.
let adjusted_step = std::cmp::max(adjusted_step, BlockRange::LENGTH);

(block_number.saturating_sub(self.security_parameter)) / adjusted_step * adjusted_step
let block_number_to_be_signed =
(block_number.saturating_sub(self.security_parameter)) / adjusted_step * adjusted_step;
block_number_to_be_signed.saturating_sub(1)
}
}

Expand Down Expand Up @@ -199,11 +204,11 @@ mod tests {
)
);

// The block number to be signed is 0 because the step is 15, the block number is 20, and
// The block number to be signed is 14 because the step is 15, the block number is 20, and
// the security parameter is 0.
// This is further tested in the "computing_block_number_to_be_signed" tests below.
assert_eq!(
SignedEntityType::CardanoTransactions(Epoch(1), 15),
SignedEntityType::CardanoTransactions(Epoch(1), 14),
config.time_point_to_signed_entity(
SignedEntityTypeDiscriminants::CardanoTransactions,
&time_point
Expand All @@ -220,7 +225,7 @@ mod tests {
step: 15,
}
.compute_block_number_to_be_signed(105),
105
104
);

assert_eq!(
Expand All @@ -229,7 +234,7 @@ mod tests {
step: 15,
}
.compute_block_number_to_be_signed(100),
90
89
);

assert_eq!(
Expand All @@ -238,7 +243,7 @@ mod tests {
step: 15,
}
.compute_block_number_to_be_signed(100),
15
14
);

assert_eq!(
Expand Down Expand Up @@ -271,7 +276,7 @@ mod tests {
step: BlockRange::LENGTH * 2 - 1,
}
.compute_block_number_to_be_signed(BlockRange::LENGTH * 5 + 1),
BlockRange::LENGTH * 5
BlockRange::LENGTH * 5 - 1
);

assert_eq!(
Expand All @@ -280,7 +285,7 @@ mod tests {
step: BlockRange::LENGTH * 2 + 1,
}
.compute_block_number_to_be_signed(BlockRange::LENGTH * 5 + 1),
BlockRange::LENGTH * 4
BlockRange::LENGTH * 4 - 1
);

// Adjusted step is always at least BLOCK_RANGE_LENGTH.
Expand All @@ -290,7 +295,7 @@ mod tests {
step: BlockRange::LENGTH - 1,
}
.compute_block_number_to_be_signed(BlockRange::LENGTH * 10 - 1),
BlockRange::LENGTH * 9
BlockRange::LENGTH * 9 - 1
);

assert_eq!(
Expand Down Expand Up @@ -424,7 +429,7 @@ mod tests {
SignedEntityType::MithrilStakeDistribution(beacon.epoch),
SignedEntityType::CardanoStakeDistribution(beacon.epoch),
SignedEntityType::CardanoImmutableFilesFull(beacon.clone()),
SignedEntityType::CardanoTransactions(beacon.epoch, chain_point.block_number),
SignedEntityType::CardanoTransactions(beacon.epoch, chain_point.block_number - 1),
],
signed_entity_types
);
Expand Down

0 comments on commit afef448

Please sign in to comment.