feat: upgrade to protocol 0.17.0-rc.3 - #2530
Conversation
# Conflicts: # CHANGELOG.md
|
You'll want to update to 0.17.0-rc.4 |
Yeah, we were tracking this. AFAIK it should not be too much extra work. The main reason why we need the 0.17 RC is to be able to implement the canonical objects work which is already merged, and also the allowlist changes that are coming to the node. |
eca2348 to
10f0223
Compare
juan518munoz
left a comment
juan518munoz
left a comment
There was a problem hiding this comment.
Looks good! I'll do a second review round later today.
I only have one question about how we are storing the protocol config, as far as I understand the only thing this struct takes is the faucet ID, so why don't we just store the hex representation in a plain form instead of the serialization of the struct?
| let fee_faucet_id = client | ||
| .get_protocol_config(genesis.protocol_config_commitment()) | ||
| .await? | ||
| .fee_asset_id() | ||
| .faucet_id(); |
There was a problem hiding this comment.
We do this in four different parts of the codebase, maybe we could abstract it into a helper?
| if let Some(path) = std::env::var_os("MIDEN_PROTOCOL_CONFIG") { | ||
| let bytes = std::fs::read(path)?; | ||
| let config = ProtocolConfig::read_from_bytes(&bytes)?; | ||
| client.add_protocol_config(config).await?; | ||
| } |
There was a problem hiding this comment.
like with integration test, should we fallback to the .bin config file as well?
| let account_component = AccountComponent::from_package(package.clone(), &init_data) | ||
| .map_err(|e| { | ||
| CliError::Account( | ||
| e, | ||
| format!("error instantiating component from Package {}", package.name), |
There was a problem hiding this comment.
nit: if we get the package.name beforehand we can avoid cloning the whole package:
| let account_component = AccountComponent::from_package(package.clone(), &init_data) | |
| .map_err(|e| { | |
| CliError::Account( | |
| e, | |
| format!("error instantiating component from Package {}", package.name), | |
| let package_name = package.name.clone(); | |
| let account_component = AccountComponent::from_package(package, &init_data) | |
| .map_err(|e| { | |
| CliError::Account( | |
| e, | |
| format!("error instantiating component from Package {}", package_name), |
| // The `account-id` core type as the compiler records it: a named record of two field elements. | ||
| // Its name is what the CLI's `account-id` codec matches against. | ||
| let account_id = Type::Struct(Arc::new(StructType::named( | ||
| let account_id = Type::Struct(midenc_hir_type::StructRef::Plain(Arc::new(StructType::named( |
There was a problem hiding this comment.
Let's move the import upwards to:
rust-sdk/bin/miden-cli/tests/cli.rs
Line 51 in 10f0223
| effective_from: config.effective_from().as_u32(), | ||
| protocol_config: Some(config.protocol_config().into()), | ||
| } | ||
| }), |
There was a problem hiding this comment.
We can implement From for validator config and next protocol config for clarity:
// VALIDATOR CONFIG
// ================================================================================================
impl From<&ValidatorConfig> for proto::blockchain::ValidatorConfig {
fn from(config: &ValidatorConfig) -> Self {
Self {
keys: config
.keys()
.iter()
.map(|key| proto::blockchain::ValidatorPublicKey { validator_key: key.to_bytes() })
.collect(),
quorum: config.quorum().into(),
}
}
}
// NEXT PROTOCOL CONFIG
// ================================================================================================
impl From<&NextProtocolConfig> for proto::blockchain::NextProtocolConfig {
fn from(config: &NextProtocolConfig) -> Self {
Self {
effective_from: config.effective_from().as_u32(),
protocol_config: Some(config.protocol_config().into()),
}
}
}| .await?; | ||
| ``` | ||
|
|
||
| ## Protocol configuration |
There was a problem hiding this comment.
Should we add to this section how to set up the config deriving it from the native faucet id?
A network that runs the standard protocol parameters needs only one network value: the account ID of the native fee faucet. This ID is a public network parameter. The operator publishes it together with the network endpoints:
```rust
use miden_client::account::AccountId;
use miden_client::asset::AssetId;
use miden_client::protocol_config::ProtocolConfig;
let fee_faucet_id = AccountId::from_hex("0x...")?;
let config = ProtocolConfig::current(AssetId::new_fungible(fee_faucet_id))?;
client.add_protocol_config(config).await?;
`` `There was a problem hiding this comment.
Entries need to carry the PR they are related with.
4 participants
Dependencies changes
0.17.0-rc.3and VM crates to0.32.miden-debugto0.15.miden-node-proto-buildto be pinned tod4062e64.About Protocol Config
The block header no longer carries the transaction kernel commitment or the fee faucet. It carries a commitment to a
ProtocolConfigholding both, plus the batch and block kernels and the proof verification policy. Execution and note screening need those values, and a commitment is one way, so the client has to be given the configuration and keep it.Added:
protocol_configmodule withClient::add_protocol_config,get_protocol_configandClientBuilder::protocol_config.DataStore::get_transaction_inputsreturns the configuration alongside the header.