add marketplace upgrade type #1850
Conversation
imabdulbasit
requested review from
nomaxg,
philippecamacho,
ImJeremyHe,
sveitser,
jbearer and
tbro
as code owners
imabdulbasit
changed the title
imabdulbasit
changed the title
sequencer/src/genesis.rs
Outdated
| UpgradeType::ChainConfig { chain_config } => { | ||
| base_fee = std::cmp::max(chain_config.base_fee, base_fee); | ||
| } | ||
| if let UpgradeType::ChainConfig { chain_config } = upgrade.upgrade_type { |
There was a problem hiding this comment.
So this Vec<&Upgrade> holds the actions we need to perform per type per upgrade? Is that right? So we are saying that for every upgrade we need to set base_fee to the maximum base fees of all upgrades. I think implicit in this is that all base_fee can only ever increment. Just checking my understanding. thanks!
There was a problem hiding this comment.
Vec<Upgrade> is basically array of tables from the genesis toml file. Yes, we set base fee to max base fee if genesis toml file has fee upgrades (chain config upgrades) specified. This is required by builder as builder takes base_fee parameter which can not be updated as it doesn't know about validated state
types/src/v0/impls/header.rs
Outdated
| .get(&version) | ||
| .map(|upgrade| match upgrade.upgrade_type { | ||
| match instance_state.upgrades.get(&version) { | ||
| Some(upgrade) => match upgrade.upgrade_type { | ||
| UpgradeType::ChainConfig { chain_config } => chain_config, |
There was a problem hiding this comment.
Not looking for a change here, just trying to improve my understanding. These semantics are confusing to me. Why do we have an UpgradeType:ChainConfig. Couldn't we just be upgrading between versions, and have the version define what is included in the upgrade? For example, I would imaging that the upgrade type would be Marketplace and ChainConfig upgrade would happen "under the hood" since it is a dependency of marketplace.
There was a problem hiding this comment.
hmm I think renaming this upgrade type to FeeUpgrade would be better. So, fee upgrade would require a new chain config that is provided in the toml file.
imabdulbasit
force-pushed
the
ab/marketplace-upgrade-type
branch
2 times, most recently
from
fbec576
to
67384e8
Compare
imabdulbasit
force-pushed
the
ab/marketplace-upgrade-type
branch
from
67384e8
to
2a0f407
Compare
No description provided.