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

feat(cli): Price Estimates in Aligned CLI #1577

Open
wants to merge 38 commits into
base: staging
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
c857a56
add price estimate as submission fee mechanism
PatStiles Dec 4, 2024
6040beb
rm error
PatStiles Dec 4, 2024
2e17db3
fmt
PatStiles Dec 4, 2024
4811db3
change name per cmts in #1558
PatStiles Dec 4, 2024
9e19acb
comment nit
PatStiles Dec 5, 2024
7c2d8f9
remove unneeded println
PatStiles Dec 5, 2024
d9c6b65
help section for cli command + prompt correct usage in errors
PatStiles Dec 5, 2024
b1c1f19
update doc comment
PatStiles Dec 5, 2024
ad91ac6
return error if user specifies both fee arguments are specified
PatStiles Dec 5, 2024
5f9d344
print max_fee in ethers
PatStiles Dec 5, 2024
c2742ff
doc changes
PatStiles Dec 5, 2024
233004f
adjust fee estime with respect to gas percentage bump in batcher + fmt
PatStiles Dec 5, 2024
dccfbbf
max_fee => max_fee_wei
PatStiles Dec 5, 2024
196056b
nit: types.rs network -> price estimate
PatStiles Dec 5, 2024
d0a99e0
use mutual exclusive args + fmting + fmt
PatStiles Dec 5, 2024
33b1044
remove unneeded TryFrom
PatStiles Dec 5, 2024
8cae2c9
cmt nits
PatStiles Dec 5, 2024
4e23678
make cases explicit instead of fun
PatStiles Dec 6, 2024
c481a00
Merge branch 'staging' into feat/sdk-pricing
PatStiles Dec 6, 2024
ac8e5b3
clippy
PatStiles Dec 6, 2024
620384b
docs + cmts + made function name more specific
PatStiles Dec 6, 2024
dcd00a1
clippy
PatStiles Dec 6, 2024
4482459
marcos comments
PatStiles Dec 6, 2024
4ffab99
marcos comments
PatStiles Dec 6, 2024
ff5ff80
fmt
PatStiles Dec 6, 2024
430b989
address Uri's comments + clippy + fmt
PatStiles Dec 6, 2024
61ab98c
nit
PatStiles Dec 6, 2024
e03f75a
missing name change
PatStiles Dec 6, 2024
170f3a3
gaston's comments
PatStiles Dec 6, 2024
f8d8308
naming nit
PatStiles Dec 6, 2024
ad111db
can variable names for price in DepositToBatcher that got mixed in me…
PatStiles Dec 6, 2024
13efe4d
change to 10
PatStiles Dec 9, 2024
d1e4c9e
uri's comments + nits + naming changes
PatStiles Dec 9, 2024
590c7d5
add back tests
PatStiles Dec 9, 2024
543c055
clippy
PatStiles Dec 9, 2024
5471636
fmt
PatStiles Dec 9, 2024
495787a
fix tests
PatStiles Dec 10, 2024
17c3ba8
feat: better msg
uri-99 Dec 10, 2024
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
4 changes: 2 additions & 2 deletions batcher/aligned-sdk/src/core/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,10 +21,10 @@ pub const PERCENTAGE_DIVIDER: u128 = 100;
/// Number of proofs we a batch for estimation.
/// This is the number of proofs in a batch of size n, where we set n = 32.
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
/// i.e. the user pays for the entire batch and his proof is instantly submitted.
pub const MAX_FEE_BATCH_PROOF_NUMBER: usize = 32;
pub const INSTANT_MAX_FEE_PROOF_NUMBER: usize = 1;
/// Estimated number of proofs for batch submission.
/// This corresponds to the number of proofs to compute for a default max_fee.
pub const MAX_FEE_DEFAULT_PROOF_NUMBER: usize = 10;
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
pub const DEFAULT_MAX_FEE_PROOF_NUMBER: usize = 10;
PatStiles marked this conversation as resolved.
Show resolved Hide resolved

/// Ethereum calls retry constants
pub const ETHEREUM_CALL_MIN_RETRY_DELAY: u64 = 500; // milliseconds
Expand Down
22 changes: 13 additions & 9 deletions batcher/aligned-sdk/src/core/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ pub enum AlignedError {
SubmitError(SubmitError),
VerificationError(VerificationError),
ChainIdError(ChainIdError),
MaxFeeEstimateError(MaxFeeEstimateError),
FeeEstimateError(FeeEstimateError),
FileError(FileError),
}

Expand All @@ -39,9 +39,9 @@ impl From<ChainIdError> for AlignedError {
}
}

impl From<MaxFeeEstimateError> for AlignedError {
fn from(e: MaxFeeEstimateError) -> Self {
AlignedError::MaxFeeEstimateError(e)
impl From<FeeEstimateError> for AlignedError {
fn from(e: FeeEstimateError) -> Self {
AlignedError::FeeEstimateError(e)
}
}

Expand All @@ -57,7 +57,7 @@ impl fmt::Display for AlignedError {
AlignedError::SubmitError(e) => write!(f, "Submit error: {}", e),
AlignedError::VerificationError(e) => write!(f, "Verification error: {}", e),
AlignedError::ChainIdError(e) => write!(f, "Chain ID error: {}", e),
AlignedError::MaxFeeEstimateError(e) => write!(f, "Max fee estimate error: {}", e),
AlignedError::FeeEstimateError(e) => write!(f, "Max fee estimate error: {}", e),
MarcosNicolau marked this conversation as resolved.
Show resolved Hide resolved
AlignedError::FileError(e) => write!(f, "File error: {}", e),
}
}
Expand Down Expand Up @@ -266,20 +266,24 @@ impl fmt::Display for ChainIdError {
}

#[derive(Debug)]
pub enum MaxFeeEstimateError {
pub enum FeeEstimateError {
EthereumProviderError(String),
EthereumGasPriceError(String),
PriceEstimateParseError(String),
}

impl fmt::Display for MaxFeeEstimateError {
impl fmt::Display for FeeEstimateError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self {
MaxFeeEstimateError::EthereumProviderError(e) => {
FeeEstimateError::EthereumProviderError(e) => {
write!(f, "Ethereum provider error: {}", e)
}
MaxFeeEstimateError::EthereumGasPriceError(e) => {
FeeEstimateError::EthereumGasPriceError(e) => {
write!(f, "Failed to retreive the current gas price: {}", e)
}
FeeEstimateError::PriceEstimateParseError(e) => {
write!(f, "Error parsing PriceEstimate: {}", e)
}
}
}
}
Expand Down
29 changes: 28 additions & 1 deletion batcher/aligned-sdk/src/core/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,36 @@ impl NoncedVerificationData {
// Defines an estimate price preference for the user.
#[derive(Debug, Serialize, Deserialize, Clone)]
pub enum PriceEstimate {
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
Min,
Default,
Instant,
Custom(usize),
}

impl TryFrom<String> for PriceEstimate {
type Error = String;

fn try_from(value: String) -> Result<Self, Self::Error> {
let val = match value.as_str() {
"default" => Self::Default,
"instant" => Self::Instant,
s if s.starts_with("custom") => {
let n = s
.split_whitespace()
.nth(1)
.ok_or("Failed to Parse: `number_proofs_per_batch` not supplied, correct usage \"custom <NUM_PROOFS_IN_BATCH>\" (note quotations)")?
.parse()
.map_err(|_| "Failed to Parse: Value of `number_proofs_per_batch` invalid , correct usage \"custom <NUM_PROOFS_IN_BATCH>\" (note quotations)")?;
PriceEstimate::Custom(n)
}
_ => {
return Err(
"Invalid network, possible values are: \"default\", \"instant\", \"custom <NUM_PROOFS_IN_BATCH>\""
uri-99 marked this conversation as resolved.
Show resolved Hide resolved
.to_string(),
)
}
};
Ok(val)
}
}

#[derive(Debug, Serialize, Deserialize, Clone, Default)]
Expand Down
122 changes: 27 additions & 95 deletions batcher/aligned-sdk/src/sdk.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use crate::{
core::{
constants::{
ADDITIONAL_SUBMISSION_GAS_COST_PER_PROOF, CONSTANT_GAS_COST,
MAX_FEE_BATCH_PROOF_NUMBER, MAX_FEE_DEFAULT_PROOF_NUMBER,
DEFAULT_MAX_FEE_PROOF_NUMBER, INSTANT_MAX_FEE_PROOF_NUMBER,
},
errors::{self, GetNonceError},
types::{
Expand Down Expand Up @@ -115,16 +115,15 @@ pub async fn submit_multiple_and_wait_verification(
aligned_verification_data
}

/// Returns the estimated `max_fee` depending on the batch inclusion preference of the user, based on the max priority gas price.
/// Returns the estimated `max_fee` depending on the batch inclusion preference of the user, computed based on the current gas price, and the number of proofs in a batch.
/// NOTE: The `max_fee` is computed from an rpc nodes max priority gas price.
/// To estimate the `max_fee` of a batch we use a compute the `max_fee` with respect to a batch of ~32 proofs present.
/// To estimate the `max_fee` of a batch we use a compute the `max_fee` with respect to a batch size of 1 (Instant), 10 (Default), or `number_proofs_in_batch` (Custom).
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
/// The `max_fee` estimates therefore are:
/// * `Min`: Specifies a `max_fee` equivalent to the cost of 1 proof in a 32 proof batch.
/// This estimates the lowest possible `max_fee` the user should specify for there proof with lowest priority.
/// * `Default`: Specifies a `max_fee` equivalent to the cost of 10 proofs in a 32 proof batch.
/// This estimates the `max_fee` the user should specify for inclusion within the batch.
/// * `Instant`: specifies a `max_fee` equivalent to the cost of all proofs within in a 32 proof batch.
/// This estimates the `max_fee` the user should specify to pay for the entire batch of proofs and have there proof included instantly.
/// * `Default`: Specifies a `max_fee` equivalent to the cost of paying for one proof within a batch of 10 proofs ie. 1 / 10 proofs.
/// This estimates a default `max_fee` the user should specify for including there proof within the batch.
/// * `Instant`: Specifies a `max_fee` equivalent to the cost of paying for an entire batch ensuring the user's proof is included instantly.
/// * `Custom (number_proofs_in_batch)`: Specifies a `max_fee` equivalent to the cost of paying 1 proof / `num_proofs_in_batch` allowing the user a user to estimate there `max_fee` precisely based on the `number_proofs_in_batch`.
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
///
/// # Arguments
/// * `eth_rpc_url` - The URL of the Ethereum RPC node.
/// * `estimate` - Enum specifying the type of price estimate: MIN, DEFAULT, INSTANT.
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -136,37 +135,17 @@ pub async fn submit_multiple_and_wait_verification(
pub async fn estimate_fee(
eth_rpc_url: &str,
estimate: PriceEstimate,
) -> Result<U256, errors::MaxFeeEstimateError> {
) -> Result<U256, errors::FeeEstimateError> {
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
// Price of 1 proof in 32 proof batch
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
let fee_per_proof = fee_per_proof(eth_rpc_url, MAX_FEE_BATCH_PROOF_NUMBER).await?;

let proof_price = match estimate {
PriceEstimate::Min => fee_per_proof,
PriceEstimate::Default => U256::from(MAX_FEE_DEFAULT_PROOF_NUMBER) * fee_per_proof,
PriceEstimate::Instant => U256::from(MAX_FEE_BATCH_PROOF_NUMBER) * fee_per_proof,
};
Ok(proof_price)
}

/// Returns the computed `max_fee` for a proof based on the number of proofs in a batch (`num_proofs_per_batch`) and
/// number of proofs (`num_proofs`) in that batch the user would pay for i.e (`num_proofs` / `num_proofs_per_batch`).
/// NOTE: The `max_fee` is computed from an rpc nodes max priority gas price.
/// # Arguments
/// * `eth_rpc_url` - The URL of the users Ethereum RPC node.
/// * `num_proofs` - number of proofs in a batch the user would pay for.
/// * `num_proofs_per_batch` - number of proofs within a batch.
/// # Returns
/// * The calculated `max_fee` as a `U256`.
/// # Errors
/// * `EthereumProviderError` if there is an error in the connection with the RPC provider.
/// * `EthereumGasPriceError` if there is an error retrieving the Ethereum gas price.
pub async fn compute_max_fee(
eth_rpc_url: &str,
num_proofs: usize,
num_proofs_per_batch: usize,
) -> Result<U256, errors::MaxFeeEstimateError> {
let fee_per_proof = fee_per_proof(eth_rpc_url, num_proofs_per_batch).await?;
Ok(fee_per_proof * num_proofs)
match estimate {
PriceEstimate::Default => {
suggest_fee_per_proof(eth_rpc_url, DEFAULT_MAX_FEE_PROOF_NUMBER).await
}
PriceEstimate::Instant => {
suggest_fee_per_proof(eth_rpc_url, INSTANT_MAX_FEE_PROOF_NUMBER).await
}
PriceEstimate::Custom(n) => suggest_fee_per_proof(eth_rpc_url, n).await,
}
}

/// Returns the `fee_per_proof` based on the current gas price for a batch compromised of `num_proofs_per_batch`
Expand All @@ -180,34 +159,34 @@ pub async fn compute_max_fee(
/// # Errors
/// * `EthereumProviderError` if there is an error in the connection with the RPC provider.
/// * `EthereumGasPriceError` if there is an error retrieving the Ethereum gas price.
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
pub async fn fee_per_proof(
pub async fn suggest_fee_per_proof(
eth_rpc_url: &str,
num_proofs_per_batch: usize,
) -> Result<U256, errors::MaxFeeEstimateError> {
num_proofs_in_batch: usize,
) -> Result<U256, errors::FeeEstimateError> {
let eth_rpc_provider =
Provider::<Http>::try_from(eth_rpc_url).map_err(|e: url::ParseError| {
errors::MaxFeeEstimateError::EthereumProviderError(e.to_string())
errors::FeeEstimateError::EthereumProviderError(e.to_string())
})?;
let gas_price = fetch_gas_price(&eth_rpc_provider).await?;

// Cost for estimate `num_proofs_per_batch` proofs
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
let estimated_gas_per_proof = (CONSTANT_GAS_COST
+ ADDITIONAL_SUBMISSION_GAS_COST_PER_PROOF * num_proofs_per_batch as u128)
/ num_proofs_per_batch as u128;
+ ADDITIONAL_SUBMISSION_GAS_COST_PER_PROOF * num_proofs_in_batch as u128)
/ num_proofs_in_batch as u128;

// Price of 1 proof in 32 proof batch
// Price of 1 / `num_proofs_in_batch` proof batch
let fee_per_proof = U256::from(estimated_gas_per_proof) * gas_price;

Ok(fee_per_proof)
}

async fn fetch_gas_price(
eth_rpc_provider: &Provider<Http>,
) -> Result<U256, errors::MaxFeeEstimateError> {
) -> Result<U256, errors::FeeEstimateError> {
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
let gas_price = match eth_rpc_provider.get_gas_price().await {
Ok(price) => price,
Err(e) => {
return Err(errors::MaxFeeEstimateError::EthereumGasPriceError(
return Err(errors::FeeEstimateError::EthereumGasPriceError(
e.to_string(),
))
}
Expand Down Expand Up @@ -823,50 +802,3 @@ fn save_response_json(

Ok(())
}

#[cfg(test)]
mod test {
PatStiles marked this conversation as resolved.
Show resolved Hide resolved
//Public constants for convenience
pub const HOLESKY_PUBLIC_RPC_URL: &str = "https://ethereum-holesky-rpc.publicnode.com";
use super::*;

#[tokio::test]
async fn computed_max_fee_for_larger_batch_is_smaller() {
let small_fee = compute_max_fee(HOLESKY_PUBLIC_RPC_URL, 2, 10)
.await
.unwrap();
let large_fee = compute_max_fee(HOLESKY_PUBLIC_RPC_URL, 5, 10)
.await
.unwrap();

assert!(small_fee < large_fee);
}

#[tokio::test]
async fn computed_max_fee_for_more_proofs_larger_than_for_less_proofs() {
let small_fee = compute_max_fee(HOLESKY_PUBLIC_RPC_URL, 5, 20)
.await
.unwrap();
let large_fee = compute_max_fee(HOLESKY_PUBLIC_RPC_URL, 5, 10)
.await
.unwrap();

assert!(small_fee < large_fee);
}

#[tokio::test]
async fn estimate_fee_are_larger_than_one_another() {
let min_fee = estimate_fee(HOLESKY_PUBLIC_RPC_URL, PriceEstimate::Min)
.await
.unwrap();
let default_fee = estimate_fee(HOLESKY_PUBLIC_RPC_URL, PriceEstimate::Default)
.await
.unwrap();
let instant_fee = estimate_fee(HOLESKY_PUBLIC_RPC_URL, PriceEstimate::Instant)
.await
.unwrap();

assert!(min_fee < default_fee);
assert!(default_fee < instant_fee);
}
}
53 changes: 42 additions & 11 deletions batcher/aligned/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ use std::path::PathBuf;
use std::str::FromStr;

use aligned_sdk::communication::serialization::cbor_deserialize;
use aligned_sdk::core::types::PriceEstimate;
use aligned_sdk::core::{
errors::{AlignedError, SubmitError},
types::{AlignedVerificationData, Network, ProvingSystemId, VerificationData},
};
use aligned_sdk::sdk::estimate_fee;
use aligned_sdk::sdk::get_chain_id;
use aligned_sdk::sdk::get_nonce_from_batcher;
use aligned_sdk::sdk::{deposit_to_aligned, get_balance_in_aligned};
Expand Down Expand Up @@ -107,20 +109,51 @@ pub struct SubmitArgs {
keystore_path: Option<PathBuf>,
#[arg(name = "Private key", long = "private_key")]
private_key: Option<String>,
#[arg(name = "Max Fee", long = "max_fee")]
MarcosNicolau marked this conversation as resolved.
Show resolved Hide resolved
max_fee: Option<String>, // String because U256 expects hex
#[arg(
name = "Max Fee",
long = "max_fee",
default_value = "1300000000000000" // 13_000 gas per proof * 100 gwei gas price (upper bound)
name = "Price Estimate",
long = "price_estimate",
default_value = "default",
help = "`price_estimate` allows for a user to use the aligned_sdk `estimate_fee` function to compute there `max_fee`.\n Usage: `default,`, `instant`, \"custom <NUM_PROOFS_IN_BATCH>\"",
MarcosNicolau marked this conversation as resolved.
Show resolved Hide resolved
allow_hyphen_values = true
)]
max_fee: String, // String because U256 expects hex
price_estimate: Option<String>,
#[arg(name = "Nonce", long = "nonce")]
nonce: Option<String>, // String because U256 expects hex
#[arg(
name = "The working network's name",
long = "network",
default_value = "devnet"
)]
network: NetworkArg,
network: Network,
}

impl SubmitArgs {
async fn get_max_fee(&self) -> Result<U256, AlignedError> {
// If max_fee is explicitly provided it is used first
if let Some(max_fee) = &self.max_fee {
// Inform the user if both are declared that `max_fee` is used.
if self.price_estimate.is_some() {
return Err(SubmitError::GenericError("`max_fee` and `price_estimate` are both present please specify one or the other".to_string()))?
}
return Ok(
U256::from_str(max_fee).map_err(|e| SubmitError::GenericError(e.to_string()))?
);
}

if let Some(price_est) = &self.price_estimate {
let estimate = PriceEstimate::try_from(price_est.clone())
.map_err(|e| SubmitError::GenericError(e.to_string()))?;
MarcosNicolau marked this conversation as resolved.
Show resolved Hide resolved
return estimate_fee(&self.eth_rpc_url, estimate)
.await
.map_err(AlignedError::FeeEstimateError);
}

// Fallback to default value
Ok(U256::from_str("1300000000000000")
.map_err(|e| SubmitError::GenericError(e.to_string()))?)
}
}

#[derive(Parser, Debug)]
Expand Down Expand Up @@ -277,9 +310,9 @@ async fn main() -> Result<(), AlignedError> {
SubmitError::IoError(batch_inclusion_data_directory_path.clone(), e)
})?;

let max_fee =
U256::from_dec_str(&submit_args.max_fee).map_err(|_| SubmitError::InvalidMaxFee)?;

let eth_rpc_url = submit_args.eth_rpc_url.clone();
// `max_fee` is required unless `price_estimate` is present.
MarcosNicolau marked this conversation as resolved.
Show resolved Hide resolved
let max_fee = submit_args.get_max_fee().await?;
MarcosNicolau marked this conversation as resolved.
Show resolved Hide resolved
let repetitions = submit_args.repetitions;
let connect_addr = submit_args.batcher_url.clone();

Expand Down Expand Up @@ -314,8 +347,6 @@ async fn main() -> Result<(), AlignedError> {
}
};

let eth_rpc_url = submit_args.eth_rpc_url.clone();

let chain_id = get_chain_id(eth_rpc_url.as_str()).await?;
wallet = wallet.with_chain_id(chain_id);

Expand Down Expand Up @@ -356,7 +387,7 @@ async fn main() -> Result<(), AlignedError> {

let aligned_verification_data_vec = submit_multiple(
&connect_addr,
submit_args.network.into(),
submit_args.network,
&verification_data_arr,
max_fee,
wallet.clone(),
Expand Down
Loading