-
Notifications
You must be signed in to change notification settings - Fork 43
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
ENG-771: Add upgrade-info json file to determine when upgrades are applied + rpc + cli #793
Changes from all commits
62123a9
d06a696
d37d312
99cc1a5
c7b4a88
4f55587
b373e0f
39d770e
9c37d99
50baa84
1318cc8
0f97e63
4ff912c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
// Copyright 2022-2024 Protocol Labs | ||
// SPDX-License-Identifier: Apache-2.0, MIT | ||
|
||
use std::path::PathBuf; | ||
|
||
use clap::{Args, Subcommand}; | ||
|
||
#[derive(Args, Debug)] | ||
pub struct UpgradeArgs { | ||
/// Path to the upgrade_info JSON file. | ||
#[arg(long)] | ||
pub upgrade_file: PathBuf, | ||
|
||
#[command(subcommand)] | ||
pub command: UpgradeCommands, | ||
} | ||
|
||
#[derive(Subcommand, Debug)] | ||
pub enum UpgradeCommands { | ||
AddUpgrade(AddUpgrade), | ||
} | ||
|
||
#[derive(Args, Debug)] | ||
pub struct AddUpgrade { | ||
/// the height at which to schedule the upgrade | ||
#[arg(long)] | ||
pub height: u64, | ||
|
||
/// the application version the upgrade will update to | ||
#[arg(long)] | ||
pub new_app_version: u64, | ||
|
||
/// the required cometbft version for the upgrade | ||
#[arg(long)] | ||
pub cometbft_version: String, | ||
|
||
/// whether this is a required upgrade or not. A required upgrade | ||
/// will cause the node to freeze and not process any more blocks | ||
/// if it does not have the corresponding Upgrade defined to | ||
/// migrate to that version | ||
#[arg(long)] | ||
pub required: bool, | ||
Comment on lines
+37
to
+42
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just thinking when an upgrade wouldn't be required. I know we talked about upgrades where you can download the new software and replace the existing one with it now, because the binary is backwards compatible, and it will apply the upgrade when the time comes. Why the upgrade list would know this though.. OTOH if some nodes update the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm also not sure how an optional upgrade would be handled if the node with the upgrade is started later than the height. Nothing will trigger that upgrade, so we don't know if it's been applied or not? |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
// Copyright 2022-2024 Protocol Labs | ||
// SPDX-License-Identifier: Apache-2.0, MIT | ||
|
||
use std::path::PathBuf; | ||
|
||
use fendermint_app_options::upgrade::AddUpgrade; | ||
use fendermint_app_options::upgrade::UpgradeArgs; | ||
use fendermint_app_options::upgrade::UpgradeCommands; | ||
use fendermint_vm_interpreter::fvm::upgrades::UpgradeSchedule; | ||
use fendermint_vm_message::ipc::UpgradeInfo; | ||
|
||
use crate::cmd; | ||
|
||
cmd! { | ||
UpgradeArgs(self) { | ||
let upgrade_file = self.upgrade_file.clone(); | ||
match &self.command { | ||
UpgradeCommands::AddUpgrade(args) => args.exec(upgrade_file).await, | ||
} | ||
} | ||
} | ||
|
||
cmd! { | ||
AddUpgrade(self, upgrade_file: PathBuf) { | ||
let mut us = UpgradeSchedule::get_or_create(&upgrade_file)?; | ||
us.add(UpgradeInfo::new( | ||
self.height.try_into().unwrap(), | ||
self.new_app_version, | ||
self.cometbft_version.clone(), | ||
self.required, | ||
))?; | ||
|
||
us.export_file(upgrade_file)?; | ||
|
||
Ok(()) | ||
} | ||
} |
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 am not exactly sure what an upgrade schedule is; maybe the docstring could explain a bit more.
For example I understand it can be the contents of the JSON file, but I was wondering if it could also include the versions that the node is capable of executing.