From 93a376884acbf99a02abeaf1701e239889d0e0df Mon Sep 17 00:00:00 2001 From: Artemii Gerasimovich Date: Fri, 26 Jul 2024 06:10:26 +0200 Subject: [PATCH] Builder API adjustments --- crates/builder-api/api/v0_3/builder.toml | 33 +-- crates/builder-api/src/v0_3/block_info.rs | 32 --- crates/builder-api/src/v0_3/builder.rs | 33 +-- crates/builder-api/src/v0_3/data_source.rs | 28 +-- crates/builder-api/src/v0_3/mod.rs | 1 - crates/task-impls/src/builder.rs | 23 +- crates/task-impls/src/transactions.rs | 231 +++++++----------- crates/types/src/bundle.rs | 7 +- .../src/traits/auction_results_provider.rs | 37 --- 9 files changed, 111 insertions(+), 314 deletions(-) delete mode 100644 crates/builder-api/src/v0_3/block_info.rs diff --git a/crates/builder-api/api/v0_3/builder.toml b/crates/builder-api/api/v0_3/builder.toml index 76a27deb3d..fbdd3a1c98 100644 --- a/crates/builder-api/api/v0_3/builder.toml +++ b/crates/builder-api/api/v0_3/builder.toml @@ -26,37 +26,12 @@ NAME = "hs-builder-get" DESCRIPTION = "" FORMAT_VERSION = "0.1.0" -[route.available_blocks] -PATH = ["availableblocks/:parent_hash/:view_number/:sender/:signature"] -":parent_hash" = "TaggedBase64" +[route.bundle] +PATH = ["bundle/:view_number"] ":view_number" = "Integer" -":sender" = "TaggedBase64" -":signature" = "TaggedBase64" +METHOD = "POST" DOC = """ -Get descriptions for all block candidates based on a specific parent block. - -Returns -``` -[ - "block_metadata": { - "block_hash": TaggedBase64, - "block_size": integer, - "offered_fee": integer, - }, -] -``` -""" - -[route.claim_block] -PATH = ["claimblock/:block_hash/:view_number/:sender/:signature"] -":block_hash" = "TaggedBase64" -":view_number" = "Integer" -":sender" = "TaggedBase64" -":signature" = "TaggedBase64" -DOC = """ -Get the specified block candidate. - -Returns application-specific encoded transactions type +Fetch the bundle from the builder for the specified view. """ [route.builder_address] diff --git a/crates/builder-api/src/v0_3/block_info.rs b/crates/builder-api/src/v0_3/block_info.rs deleted file mode 100644 index 56df60864b..0000000000 --- a/crates/builder-api/src/v0_3/block_info.rs +++ /dev/null @@ -1,32 +0,0 @@ -use hotshot_types::traits::{ - node_implementation::NodeType, signature_key::BuilderSignatureKey, BlockPayload, -}; -use serde::{Deserialize, Serialize}; - -/// No changes to these types -pub use crate::v0_1::block_info::AvailableBlockInfo; - -#[derive(Clone, Debug, Deserialize, Serialize, PartialEq, Eq, Hash)] -#[serde(bound = "")] -pub struct AvailableBlockData { - pub block_payload: TYPES::BlockPayload, - pub metadata: >::Metadata, - pub fee: u64, - pub signature: - <::BuilderSignatureKey as BuilderSignatureKey>::BuilderSignature, - pub fee_signature: - <::BuilderSignatureKey as BuilderSignatureKey>::BuilderSignature, - pub sender: ::BuilderSignatureKey, -} - -impl AvailableBlockData { - pub fn validate_signature(&self) -> bool { - // verify the signature over the message, construct the builder commitment - let builder_commitment = self.block_payload.builder_commitment(&self.metadata); - self.sender - .validate_builder_signature(&self.signature, builder_commitment.as_ref()) - && self - .sender - .validate_sequencing_fee_signature_marketplace(&self.fee_signature, self.fee) - } -} diff --git a/crates/builder-api/src/v0_3/builder.rs b/crates/builder-api/src/v0_3/builder.rs index d5ebad46fa..b064fd84af 100644 --- a/crates/builder-api/src/v0_3/builder.rs +++ b/crates/builder-api/src/v0_3/builder.rs @@ -1,15 +1,15 @@ use futures::FutureExt; -use hotshot_types::{traits::node_implementation::NodeType, utils::BuilderCommitment}; +use hotshot_types::traits::node_implementation::NodeType; use snafu::ResultExt; use tide_disco::{api::ApiError, method::ReadState, Api}; use super::{data_source::BuilderDataSource, Version}; +use crate::api::load_api; /// No changes to these types pub use crate::v0_1::builder::{ submit_api, BlockAvailableSnafu, BlockClaimSnafu, BuildError, BuilderAddressSnafu, Error, Options, }; -use crate::{api::load_api, v0_1::builder::try_extract_param}; pub fn define_api( options: &Options, @@ -24,33 +24,12 @@ where options.extensions.clone(), )?; api.with_version("0.0.3".parse().unwrap()) - .get("available_blocks", |req, state| { + .get("bundle", |req, state| { async move { - let hash = req.blob_param("parent_hash")?; let view_number = req.integer_param("view_number")?; - let signature = try_extract_param(&req, "signature")?; - let sender = try_extract_param(&req, "sender")?; - state - .available_blocks(&hash, view_number, sender, &signature) - .await - .context(BlockAvailableSnafu { - resource: hash.to_string(), - }) - } - .boxed() - })? - .get("claim_block", |req, state| { - async move { - let block_hash: BuilderCommitment = req.blob_param("block_hash")?; - let view_number = req.integer_param("view_number")?; - let signature = try_extract_param(&req, "signature")?; - let sender = try_extract_param(&req, "sender")?; - state - .claim_block(&block_hash, view_number, sender, &signature) - .await - .context(BlockClaimSnafu { - resource: block_hash.to_string(), - }) + state.bundle(view_number).await.context(BlockClaimSnafu { + resource: view_number.to_string(), + }) } .boxed() })? diff --git a/crates/builder-api/src/v0_3/data_source.rs b/crates/builder-api/src/v0_3/data_source.rs index 8face03e42..d37acd3dde 100644 --- a/crates/builder-api/src/v0_3/data_source.rs +++ b/crates/builder-api/src/v0_3/data_source.rs @@ -1,36 +1,14 @@ use async_trait::async_trait; -use hotshot_types::{ - traits::{node_implementation::NodeType, signature_key::SignatureKey}, - utils::BuilderCommitment, - vid::VidCommitment, -}; +use hotshot_types::{bundle::Bundle, traits::node_implementation::NodeType}; -use super::{ - block_info::{AvailableBlockData, AvailableBlockInfo}, - builder::BuildError, -}; +use super::builder::BuildError; /// No changes to these types pub use crate::v0_1::data_source::AcceptsTxnSubmits; #[async_trait] pub trait BuilderDataSource { /// To get the list of available blocks - async fn available_blocks( - &self, - for_parent: &VidCommitment, - view_number: u64, - sender: TYPES::SignatureKey, - signature: &::PureAssembledSignatureType, - ) -> Result>, BuildError>; - - /// to claim a block from the list of provided available blocks - async fn claim_block( - &self, - block_hash: &BuilderCommitment, - view_number: u64, - sender: TYPES::SignatureKey, - signature: &::PureAssembledSignatureType, - ) -> Result, BuildError>; + async fn bundle(&self, view_number: u64) -> Result, BuildError>; /// To get the builder's address async fn builder_address(&self) -> Result; diff --git a/crates/builder-api/src/v0_3/mod.rs b/crates/builder-api/src/v0_3/mod.rs index 21b42d83e0..b691543cb9 100644 --- a/crates/builder-api/src/v0_3/mod.rs +++ b/crates/builder-api/src/v0_3/mod.rs @@ -1,4 +1,3 @@ -pub mod block_info; pub mod builder; pub mod data_source; /// No changes to this module diff --git a/crates/task-impls/src/builder.rs b/crates/task-impls/src/builder.rs index df7fd5f661..f5b5b1261d 100644 --- a/crates/task-impls/src/builder.rs +++ b/crates/task-impls/src/builder.rs @@ -194,16 +194,10 @@ pub mod v0_2 { pub type Version = StaticVersion<0, 2>; } -/// Version 0.3. Removes `claim_block_header_input` endpoint, adds fee information -/// to `claim_block` endpoint. +/// Version 0.3: marketplace. Bundles. pub mod v0_3 { - use hotshot_builder_api::v0_3::block_info::AvailableBlockData; pub use hotshot_builder_api::v0_3::Version; - use hotshot_types::{ - traits::{node_implementation::NodeType, signature_key::SignatureKey}, - utils::BuilderCommitment, - }; - use tagged_base64::TaggedBase64; + use hotshot_types::{bundle::Bundle, traits::node_implementation::NodeType}; use vbs::version::StaticVersion; pub use super::BuilderClientError; @@ -217,18 +211,9 @@ pub mod v0_3 { /// # Errors /// - [`BuilderClientError::NotFound`] if block isn't available /// - [`BuilderClientError::Api`] if API isn't responding or responds incorrectly - pub async fn claim_block( - &self, - block_hash: BuilderCommitment, - view_number: u64, - sender: TYPES::SignatureKey, - signature: &<::SignatureKey as SignatureKey>::PureAssembledSignatureType, - ) -> Result, BuilderClientError> { - let encoded_signature: TaggedBase64 = signature.clone().into(); + pub async fn bundle(&self, view_number: u64) -> Result, BuilderClientError> { self.inner - .get(&format!( - "claimblock/{block_hash}/{view_number}/{sender}/{encoded_signature}" - )) + .get(&format!("bundle/{view_number}")) .send() .await .map_err(Into::into) diff --git a/crates/task-impls/src/transactions.rs b/crates/task-impls/src/transactions.rs index e912b5a42d..1885bdaf3a 100644 --- a/crates/task-impls/src/transactions.rs +++ b/crates/task-impls/src/transactions.rs @@ -5,10 +5,10 @@ use std::{ use anyhow::{bail, Result}; use async_broadcast::{Receiver, Sender}; -use async_compatibility_layer::art::async_sleep; +use async_compatibility_layer::art::{async_sleep, async_timeout}; use async_lock::RwLock; use async_trait::async_trait; -use futures::{stream::FuturesUnordered, StreamExt}; +use futures::{future::join_all, stream::FuturesUnordered, StreamExt}; use hotshot_builder_api::v0_1::block_info::AvailableBlockInfo; use hotshot_task::task::TaskState; use hotshot_types::{ @@ -18,7 +18,7 @@ use hotshot_types::{ event::{Event, EventType}, simple_certificate::{version, UpgradeCertificate}, traits::{ - auction_results_provider::AuctionResultsProvider, + auction_results_provider::{AuctionResultsProvider, HasUrls}, block_contents::{precompute_vid_commitment, BuilderFee, EncodeBytes}, election::Membership, node_implementation::{ConsensusTime, NodeImplementation, NodeType}, @@ -29,7 +29,7 @@ use hotshot_types::{ vid::{VidCommitment, VidPrecomputeData}, }; use tracing::{debug, error, instrument, warn}; -use vbs::version::{StaticVersionType, Version}; +use vbs::version::StaticVersionType; use vec1::Vec1; use crate::{ @@ -143,7 +143,7 @@ impl> TransactionTaskState> TransactionTaskState bundles.push(b), + _ => continue, + } + } + let mut sequencing_fees = Vec::new(); let mut transactions: Vec< >::Transaction, @@ -435,11 +460,7 @@ impl> TransactionTaskState Option> { + async fn wait_for_block(&self, block_view: TYPES::Time) -> Option> { let task_start_time = Instant::now(); // Find commitment to the block we want to build upon @@ -470,10 +491,10 @@ impl> TransactionTaskState> TransactionTaskState::SignatureKey as SignatureKey>::PureAssembledSignatureType, - version: Version, ) -> Vec<(AvailableBlockInfo, usize)> { - /// Implementations between versions are essentially the same except for the builder - /// clients used. The most conscise way to express this is with a macro. - macro_rules! inner_impl { - ($clients:ident) => {{ - // Create a collection of futures that call available_blocks endpoint for every builder - let tasks = self - .$clients - .iter() - .enumerate() - .map(|(builder_idx, client)| async move { - client - .available_blocks( - parent_comm, - view_number.u64(), - self.public_key.clone(), - parent_comm_sig, - ) - .await - .map(move |blocks| { - // Add index into `self.builder_clients` for each block so that we know - // where to claim it from later - blocks - .into_iter() - .map(move |block_info| (block_info, builder_idx)) - }) - }) - .collect::>(); - - // A vector of resolved builder responses - let mut results = Vec::with_capacity(self.$clients.len()); - - // Instant we start querying builders for available blocks - let query_start = Instant::now(); - - // First we complete the query to the fastest fraction of the builders - let threshold = (self.$clients.len() * BUILDER_MAIN_BATCH_THRESHOLD_DIVIDEND) - .div_ceil(BUILDER_MAIN_BATCH_THRESHOLD_DIVISOR); - let mut tasks = tasks.take(threshold); - while let Some(result) = tasks.next().await { - results.push(result); - if query_start.elapsed() > BUILDER_MAIN_BATCH_CUTOFF { - break; - } - } - - // Then we query the rest, alotting additional `elapsed * BUILDER_ADDITIONAL_TIME_MULTIPLIER` - // for them to respond. There's a fixed floor of `BUILDER_MINIMUM_QUERY_TIME` for both - // phases - let timeout = async_sleep(std::cmp::max( - query_start - .elapsed() - .mul_f32(BUILDER_ADDITIONAL_TIME_MULTIPLIER), - BUILDER_MINIMUM_QUERY_TIME.saturating_sub(query_start.elapsed()), - )); - futures::pin_mut!(timeout); // Stream::next requires Self::Unpin - let mut tasks = tasks.into_inner().take_until(timeout); - while let Some(result) = tasks.next().await { - results.push(result); - } - - results - .into_iter() - .filter_map(|result| match result { - Ok(value) => Some(value), - Err(err) => { - tracing::warn!(%err, "Error getting available blocks"); - None - } + let tasks = self + .builder_clients + .iter() + .enumerate() + .map(|(builder_idx, client)| async move { + client + .available_blocks( + parent_comm, + view_number.u64(), + self.public_key.clone(), + parent_comm_sig, + ) + .await + .map(move |blocks| { + blocks + .into_iter() + .map(move |block_info| (block_info, builder_idx)) }) - .flatten() - .collect::>() - }} + }) + .collect::>(); + let mut results = Vec::with_capacity(self.builder_clients.len()); + let query_start = Instant::now(); + let threshold = (self.builder_clients.len() * BUILDER_MAIN_BATCH_THRESHOLD_DIVIDEND) + .div_ceil(BUILDER_MAIN_BATCH_THRESHOLD_DIVISOR); + let mut tasks = tasks.take(threshold); + while let Some(result) = tasks.next().await { + results.push(result); + if query_start.elapsed() > BUILDER_MAIN_BATCH_CUTOFF { + break; + } } - - if version >= MarketplaceVersion::version() { - inner_impl!(builder_clients_marketplace) - } else { - inner_impl!(builder_clients) + let timeout = async_sleep(std::cmp::max( + query_start + .elapsed() + .mul_f32(BUILDER_ADDITIONAL_TIME_MULTIPLIER), + BUILDER_MINIMUM_QUERY_TIME.saturating_sub(query_start.elapsed()), + )); + futures::pin_mut!(timeout); + let mut tasks = tasks.into_inner().take_until(timeout); + while let Some(result) = tasks.next().await { + results.push(result); } + results + .into_iter() + .filter_map(|result| match result { + Ok(value) => Some(value), + Err(err) => { + tracing::warn!(%err,"Error getting available blocks"); + None + } + }) + .flatten() + .collect::>() } /// Get a block from builder. @@ -605,10 +599,9 @@ impl> TransactionTaskState::SignatureKey as SignatureKey>::PureAssembledSignatureType, - version: Version, ) -> anyhow::Result> { let mut available_blocks = self - .get_available_blocks(parent_comm, view_number, parent_comm_sig, version) + .get_available_blocks(parent_comm, view_number, parent_comm_sig) .await; available_blocks.sort_by(|(l, _), (r, _)| { @@ -650,53 +643,7 @@ impl> TransactionTaskState= MarketplaceVersion::version() { - let client = &self.builder_clients_marketplace[builder_idx]; - - let block = client - .claim_block( - block_info.block_hash.clone(), - view_number.u64(), - self.public_key.clone(), - &request_signature, - ) - .await; - - let block_data = match block { - Ok(block_data) => block_data, - Err(err) => { - tracing::warn!(%err, "Error claiming block data"); - continue; - } - }; - - // verify the signature over the message, construct the builder commitment - let builder_commitment = block_data - .block_payload - .builder_commitment(&block_data.metadata); - if !block_data - .sender - .validate_builder_signature(&block_data.signature, builder_commitment.as_ref()) - { - tracing::warn!( - "Failed to verify available block data response message signature" - ); - continue; - } - - let fee = BuilderFee { - fee_amount: block_info.offered_fee, - fee_account: block_data.sender, - fee_signature: block_data.signature, - }; - - BuilderResponse { - fee, - block_payload: block_data.block_payload, - metadata: block_data.metadata, - precompute_data: None, - } - } else { + let response = { let client = &self.builder_clients[builder_idx]; let (block, header_input) = futures::join! { diff --git a/crates/types/src/bundle.rs b/crates/types/src/bundle.rs index 81bf666b98..e211772557 100644 --- a/crates/types/src/bundle.rs +++ b/crates/types/src/bundle.rs @@ -2,7 +2,10 @@ use serde::{Deserialize, Serialize}; -use crate::traits::{block_contents::BuilderFee, node_implementation::NodeType, BlockPayload}; +use crate::traits::{ + block_contents::BuilderFee, node_implementation::NodeType, signature_key::BuilderSignatureKey, + BlockPayload, +}; #[derive(Clone, Debug, Serialize, Deserialize)] #[serde(bound = "TYPES: NodeType")] @@ -14,7 +17,7 @@ pub struct Bundle { pub transactions: Vec<>::Transaction>, /// The signature over the bundle. - pub signature: TYPES::SignatureKey, + pub signature: ::BuilderSignature, /// The fee for sequencing pub sequencing_fee: BuilderFee, diff --git a/crates/types/src/traits/auction_results_provider.rs b/crates/types/src/traits/auction_results_provider.rs index aa27925510..1d373e2af2 100644 --- a/crates/types/src/traits/auction_results_provider.rs +++ b/crates/types/src/traits/auction_results_provider.rs @@ -2,16 +2,10 @@ //! which handles connecting to, and fetching the allocation results from, the Solver. use anyhow::Result; -use async_compatibility_layer::art::async_timeout; use async_trait::async_trait; -use futures::future::join_all; use url::Url; use super::node_implementation::NodeType; -use crate::{ - bundle::Bundle, - constants::{AUCTION_RESULTS_FETCH_TIMEOUT, BUNDLE_FETCH_TIMEOUT}, -}; /// This trait guarantees that a particular type has urls that can be extracted from it. This trait /// essentially ensures that the results returned by the [`AuctionResultsProvider`] trait includes a @@ -34,35 +28,4 @@ pub trait AuctionResultsProvider: Send + Sync + Clone { /// Fetches the auction result for a view. Does not cache the result, /// subsequent calls will invoke additional wasted calls. async fn fetch_auction_result(&self, view_number: TYPES::Time) -> Result; - - /// Fetches the bundles for a view. - async fn fetch_bundles(&self, view_number: TYPES::Time) -> Result>> { - let result = async_timeout( - AUCTION_RESULTS_FETCH_TIMEOUT, - self.fetch_auction_result(view_number), - ) - .await??; - - let client = reqwest::Client::new(); - - let mut futures = Vec::new(); - - for url in result.urls() { - futures.push(async_timeout( - BUNDLE_FETCH_TIMEOUT, - client.get(url).send().await?.json::>(), - )); - } - - let mut bundles = Vec::new(); - - for bundle in join_all(futures).await { - match bundle { - Ok(Ok(b)) => bundles.push(b), - _ => continue, - } - } - - Ok(bundles) - } }