-
Notifications
You must be signed in to change notification settings - Fork 87
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
Update to hotshot rc-0.5.67 / marketplace #1784
Conversation
3214997
to
f04b89f
Compare
when validating builder_fees, try both v1 and v3 signature schems, if either succeeds, pass validation
}, | ||
// TODO does this work? is there a better way? | ||
Either::Right(c) => ResolvableChainConfig { | ||
chain_config: Either::Right(Commitment::from_str(&c.to_string()).unwrap()), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also haven't found a better way to handle this. maybe @jbearer would know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is ok to merge for now, but we try to figure out if there is a better way before we turn on marketplace.
types/src/v0/v0_3/chain_config.rs
Outdated
fn default() -> Self { | ||
Self { | ||
chain_id: U256::from(35353).into(), // arbitrarily chosen chain ID | ||
max_block_size: 10240.into(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dev node test will fail. I had to change it to 30mb
types/src/v0/v0_1/chain_config.rs
Outdated
fn default() -> Self { | ||
Self { | ||
chain_id: U256::from(35353).into(), // arbitrarily chosen chain ID | ||
max_block_size: 10240.into(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here
} | ||
} | ||
|
||
const SOLVER_URL: &str = "https://solver:1234"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should come from env var?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes but it doesn't matter yet.
data/genesis/demo.toml
Outdated
start_proposing_view = 5 | ||
stop_proposing_view = 15 | ||
|
||
[upgrade.marketplace] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can remove it for now as UpgradeType changes are in integrations PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that toml parsing will fail so yeah needs to be removed
types/src/v0/impls/auction.rs
Outdated
@@ -137,6 +150,9 @@ pub enum ExecutionError { | |||
#[error("Could not resolve `ChainConfig`")] | |||
/// Could not resolve `ChainConfig`. | |||
UnresolvableChainConfig, | |||
#[error("Bid ricipient not set on `ChainConfig`")] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo here: Bid recipient
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! left some minor comments.
Update to hotshot rc-0.5.66
Most changes are being brought in from #1696. Also includes changes
from #1781. Most compile errors are fixed. Depends on
hotshot-query-service
pointing to the same hotshot tab (seeCargo.toml patch).