Skip to content

Commit

Permalink
fix(ckbtc): state schema upgrade (#3071)
Browse files Browse the repository at this point in the history
Ensure that changes to the state schema, which is stored in stable
memory, are backwards compatible.

---------

Co-authored-by: Paul H. Liu <[email protected]>
  • Loading branch information
gregorydemay and ninegua authored Dec 10, 2024
1 parent b937ba1 commit 57362c3
Show file tree
Hide file tree
Showing 6 changed files with 91 additions and 5 deletions.
3 changes: 2 additions & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions rs/bitcoin/checker/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ rust_test(
# Keep sorted.
"@crate_index//:candid_parser",
"@crate_index//:ic-btc-interface",
"@crate_index//:proptest",
"@crate_index//:scraper",
"@crate_index//:tokio",
],
Expand Down
3 changes: 2 additions & 1 deletion rs/bitcoin/checker/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ ic-base-types = { path = "../../types/base_types" }
ic-types = { path = "../../types/types" }
ic-test-utilities-load-wasm = { path = "../../test_utilities/load_wasm" }
ic-universal-canister = { path = "../../universal_canister/lib" }
pocket-ic = { path = "../../../packages/pocket-ic" }
pocket-ic = { path = "../../../packages/pocket-ic" }
proptest = { workspace = true }
tokio = { workspace = true }
scraper = "0.17.1"
3 changes: 1 addition & 2 deletions rs/bitcoin/checker/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,13 +119,12 @@ fn transform(raw: TransformArgs) -> HttpResponse {

#[ic_cdk::init]
fn init(arg: CheckArg) {
const DEFAULT_NUM_SUBNET_NODES: u16 = 34;
match arg {
CheckArg::InitArg(init_arg) => set_config(
Config::new_and_validate(
init_arg.btc_network,
init_arg.check_mode,
DEFAULT_NUM_SUBNET_NODES,
state::default_num_subnet_nodes(),
)
.unwrap_or_else(|err| ic_cdk::trap(&format!("error creating config: {}", err))),
),
Expand Down
7 changes: 6 additions & 1 deletion rs/bitcoin/checker/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,13 +278,18 @@ impl Drop for FetchGuard {
}
}

#[derive(Clone, Debug, Deserialize, Serialize)]
#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
pub struct Config {
btc_network: BtcNetwork,
pub check_mode: CheckMode,
#[serde(default = "default_num_subnet_nodes")]
pub num_subnet_nodes: u16,
}

pub(crate) fn default_num_subnet_nodes() -> u16 {
34
}

impl Config {
pub fn new_and_validate(
btc_network: BtcNetwork,
Expand Down
79 changes: 79 additions & 0 deletions rs/bitcoin/checker/src/state/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -154,3 +154,82 @@ fn cache_should_have_same_size() {
assert_eq!(cache.status.len(), 1);
assert_eq!(cache.created.len(), 1);
}

mod schema_upgrades {
use crate::state::Config;
use candid::Deserialize;
use ic_btc_checker::{BtcNetwork, CheckMode};
use proptest::arbitrary::any;
use proptest::prelude::{Just, Strategy};
use proptest::{prop_oneof, proptest};
use serde::Serialize;

#[derive(Clone, Debug, PartialEq, Eq, Deserialize, Serialize)]
pub struct ConfigPreviousVersion {
btc_network: BtcNetwork,
check_mode: CheckMode,
}

impl From<Config> for ConfigPreviousVersion {
fn from(
Config {
btc_network,
check_mode,
num_subnet_nodes: _,
}: Config,
) -> Self {
Self {
btc_network,
check_mode,
}
}
}

proptest! {
#[test]
fn should_be_able_to_upgrade_state(config in arb_config()) {
let config_before_upgrade: ConfigPreviousVersion = config.into();

let serialized_config_before_upgrade = encode(&config_before_upgrade);
let config_after_upgrade: Config = decode(serialized_config_before_upgrade.as_slice());
assert_eq!(config_before_upgrade, config_after_upgrade.clone().into());
}
}

fn encode<S: ?Sized + serde::Serialize>(state: &S) -> Vec<u8> {
let mut buf = vec![];
ciborium::ser::into_writer(state, &mut buf).expect("failed to encode state");
buf
}

fn decode<T: serde::de::DeserializeOwned>(bytes: &[u8]) -> T {
ciborium::de::from_reader(bytes)
.unwrap_or_else(|e| panic!("failed to decode state bytes {}: {e}", hex::encode(bytes)))
}

pub fn arb_config() -> impl Strategy<Value = Config> {
(arb_btc_network(), arb_check_mode(), any::<u16>()).prop_map(
|(btc_network, check_mode, num_subnet_nodes)| Config {
btc_network,
check_mode,
num_subnet_nodes,
},
)
}

pub fn arb_btc_network() -> impl Strategy<Value = BtcNetwork> {
prop_oneof![
Just(BtcNetwork::Mainnet),
Just(BtcNetwork::Testnet),
".*".prop_map(|json_rpc_url| BtcNetwork::Regtest { json_rpc_url }),
]
}

pub fn arb_check_mode() -> impl Strategy<Value = CheckMode> {
prop_oneof![
Just(CheckMode::AcceptAll),
Just(CheckMode::RejectAll),
Just(CheckMode::Normal),
]
}
}

0 comments on commit 57362c3

Please sign in to comment.