Repository navigation
feat: adds host simulation to block build - #167
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
a2adc44 to
9b8137b
Compare
9b8137b to
40a2b56
Compare
59afcac to
cf31bc9
Compare
0c07c93 to
eb5a2f7
Compare
84b510e to
6166fcb
Compare
cf31bc9 to
7600937
Compare
Merge activity
|
- properly builds a cfg env and block env from prev host header - makes max host gas a configurable property - cleans up the build prep in the sim loop
* fix: move block env creation for host to sim task * rename vars to clearly distinguish between rollup and host (#170) * comment --------- Co-authored-by: Anna Carroll <anna-carroll@users.noreply.github.com> Co-authored-by: Anna Carroll <anna.s.carroll@gmail.com>
b6d52e8 to
9c61587
Compare
| #[from_env( | ||
| var = "MAX_HOST_GAS_COEFFICIENT", | ||
| desc = "Optional maximum host gas coefficient, as a percentage, to use when building blocks", | ||
| default = 80 |
There was a problem hiding this comment.
should we make this number a constant?
There was a problem hiding this comment.
if this is a percentage, it should be f64, and explicitly capped to 99.9% (due to eth consensus rules on limit changes
| /// Returns the maximum host gas to use for block building based on the configured max host gas coefficient. | ||
| pub fn max_host_gas(&self, gas_limit: u64) -> u64 { | ||
| // Set max host gas to a percentage of the host block gas limit | ||
| ((gas_limit as u128 * (self.max_host_gas_coefficient.unwrap_or(80) as u128)) / 100u128) |
There was a problem hiding this comment.
ditto, this should be a constant so there's only 1 place that needs to change if we decide to change the coefficient
There was a problem hiding this comment.
honestly, I think that this should not be configurable right now. instead of having this be a config struct prop, we could just make it a constant 80%. we can make it configurable later if we want to
There was a problem hiding this comment.
yeah i'm happy with that, I think it's safer that way and makes other builders understand that changing this means you're running modified software
| 1740681556, // pecorino start timestamp as sane default | ||
| 0, 1, | ||
| ), | ||
| max_host_gas_coefficient: Some(80), |
| @@ -118,7 +276,7 @@ impl EnvTask { | |||
| span, | |||
| error!("error fetching previous host block - skipping block submission") | |||
There was a problem hiding this comment.
would love someone to check my logic here:
it’s occurring to me that if the Builder is configured to use a Host RPC served by a different Node than is running the Rollup ExEx and serving the Rollup RPC, then they are more likely to be out of sync which would cause reliability issues in the Env task
in testnet this is impossible to configure incorrectly because there is only one Host RPC and it’s the exact same Node as is running the ExEx for the Rollup RPC, but in mainnet it would be easy to configure incorrectly
if i'm right we should document this configuration need for Builders.
There was a problem hiding this comment.
also, possibly add some retry logic for the Host RPC call
feat: adds host simulation to block build
This PR adds host simulation to the block build loop.
0.15.1BuilderHelper wrapper contract submission in favor of the rawZenith contractHostProvider into the Simulator for host transaction simulation