Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: correct threshold for light client state update #1461

Merged
merged 7 commits into from
May 16, 2024
Merged
Show file tree
Hide file tree
Changes from 6 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
18 changes: 10 additions & 8 deletions hotshot-state-prover/src/mock_ledger.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ use hotshot_contract_adapter::jellyfish::{field_to_u256, open_key, u256_to_field
use hotshot_contract_adapter::light_client::ParsedLightClientState;
use hotshot_stake_table::vec_based::StakeTable;

use crate::{generate_state_update_proof, preprocess, Proof, VerifyingKey};
use crate::{
generate_state_update_proof, preprocess, service::compute_quorum_threshold, Proof, VerifyingKey,
};
use hotshot_types::traits::stake_table::StakeTableScheme;
use hotshot_types::{
light_client::{GenericLightClientState, GenericPublicInput, LightClientState},
Expand Down Expand Up @@ -86,7 +88,8 @@ impl MockLedger {
key_archive.insert(qc_keys[i], state_keys[i].0.clone());
}
let st = stake_table_for_testing(&qc_keys, &state_keys);
let threshold = st.total_stake(SnapshotVersion::LastEpochStart).unwrap() * 2 / 3;
let threshold =
compute_quorum_threshold(st.total_stake(SnapshotVersion::LastEpochStart).unwrap());

// arbitrary commitment values as they don't affect logic being tested
let block_comm_root = F::from(1234);
Expand Down Expand Up @@ -120,12 +123,11 @@ impl MockLedger {
{
self.epoch += 1;
self.st.advance();
self.threshold = self
.st
.total_stake(SnapshotVersion::LastEpochStart)
.unwrap()
* 2
/ 3;
self.threshold = compute_quorum_threshold(
self.st
.total_stake(SnapshotVersion::LastEpochStart)
.unwrap(),
);
}

let new_root = self.new_dummy_comm();
Expand Down
10 changes: 8 additions & 2 deletions hotshot-state-prover/src/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,12 @@ pub struct StateProverConfig {
pub stake_table_capacity: usize,
}

#[inline]
/// A helper function to compute the quorum threshold given a total amount of stake.
pub fn compute_quorum_threshold(total_stake: U256) -> U256 {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should call this one_honest_threshold, not quorum

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right: 0a6b8b1

total_stake / 3 + 1
}

pub fn init_stake_table(
bls_keys: &[BLSPubKey],
state_keys: &[StateVerKey],
Expand Down Expand Up @@ -156,7 +162,7 @@ pub async fn light_client_genesis(
let (bls_comm, schnorr_comm, stake_comm) = st
.commitment(SnapshotVersion::LastEpochStart)
.expect("Commitment computation shouldn't fail.");
let threshold = st.total_stake(SnapshotVersion::LastEpochStart)? * 2 / 3;
let threshold = compute_quorum_threshold(st.total_stake(SnapshotVersion::LastEpochStart)?);

let pi = vec![
u256_to_field(threshold),
Expand Down Expand Up @@ -297,7 +303,7 @@ pub async fn sync_state<Ver: StaticVersionType>(
tracing::debug!("Old state: {old_state:?}");
tracing::debug!("New state: {:?}", bundle.state);

let threshold = st.total_stake(SnapshotVersion::LastEpochStart)? * 2 / 3;
let threshold = compute_quorum_threshold(st.total_stake(SnapshotVersion::LastEpochStart)?);
tracing::info!("Threshold before syncing state: {}", threshold);
let entries = st
.try_iter(SnapshotVersion::LastEpochStart)
Expand Down
2 changes: 1 addition & 1 deletion sequencer/src/bin/deploy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ struct Options {
#[clap(flatten)]
contracts: DeployedContracts,

/// If toggled, launch a mock prover contract that does not do any proof verification.
/// If toggled, launch a mock prover contract with a smaller verification key.
#[clap(short, long)]
pub use_mock_contract: bool,

Expand Down
9 changes: 7 additions & 2 deletions sequencer/src/bin/state-relay-server.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use async_compatibility_layer::logging::{setup_backtrace, setup_logging};
use clap::Parser;
use es_version::SEQUENCER_VERSION;
use ethers::types::U256;
use hotshot_state_prover::service::compute_quorum_threshold;
use sequencer::state_signature::relay_server::run_relay_server;

#[derive(Parser)]
Expand Down Expand Up @@ -31,9 +33,12 @@ async fn main() {
setup_backtrace();

let args = Args::parse();
let threshold = ((2 * args.total_stake) / 3) + 1;
let threshold = compute_quorum_threshold(U256::from(args.total_stake));

tracing::info!(port = args.port, threshold, "starting state relay server");
tracing::info!(
port = args.port,
"starting state relay server, quorum threshold: {threshold}"
);
run_relay_server(
None,
threshold,
Expand Down
3 changes: 1 addition & 2 deletions sequencer/src/state_signature/relay_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ where

pub async fn run_relay_server<Ver: StaticVersionType + 'static>(
shutdown_listener: Option<OneShotReceiver<()>>,
threshold: u64,
threshold: U256,
url: Url,
bind_version: Ver,
) -> std::io::Result<()> {
Expand All @@ -234,7 +234,6 @@ pub async fn run_relay_server<Ver: StaticVersionType + 'static>(

// We don't have a stake table yet, putting some temporary value here.
// Related issue: [https://github.com/EspressoSystems/espresso-sequencer/issues/1022]
let threshold = U256::from(threshold);
let state =
State::new(StateRelayServerState::new(threshold).with_shutdown_signal(shutdown_listener));
let mut app = App::<State, Error>::with_state(state);
Expand Down
Loading