Skip to content
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

fix (EXC-1590): make burning cycles configurable #286

Merged
merged 6 commits into from
Apr 3, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,6 @@
target
.dfx
canister_ids.json
.canbench

*.wasm.gz
2 changes: 2 additions & 0 deletions canister/candid.did
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ type config = record {
api_access : flag;
disable_api_if_not_fully_synced : flag;
watchdog_canister : opt principal;
burn_cycles : flag;
dsarlis marked this conversation as resolved.
Show resolved Hide resolved
};

type fees = record {
Expand Down Expand Up @@ -89,6 +90,7 @@ type set_config_request = record {
api_access : opt flag;
disable_api_if_not_fully_synced : opt flag;
watchdog_canister : opt opt principal;
burn_cycles : opt flag;
};

service bitcoin : (config) -> {
Expand Down
12 changes: 9 additions & 3 deletions canister/src/heartbeat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,7 @@ use ic_btc_types::{Block, BlockHash};
pub async fn heartbeat() {
print("Starting heartbeat...");

// Burn any cycles in the canister's balance (to count towards the IC's cycles burn rate)
let cycles_burnt = cycles_burn();
add_cycles_burnt_to_metric(cycles_burnt);
maybe_burn_cycles();

if ingest_stable_blocks_into_utxoset() {
// Exit the heartbeat if stable blocks had been ingested.
Expand Down Expand Up @@ -243,6 +241,14 @@ fn add_cycles_burnt_to_metric(cycles_burnt: u128) {
});
}

/// Burns any cycles in the canister's balance (to count towards the IC's cycles burn rate).
fn maybe_burn_cycles() {
if with_state(|s| s.burn_cycles == Flag::Enabled) {
let cycles_burnt = cycles_burn();
add_cycles_burnt_to_metric(cycles_burnt);
}
}

#[cfg(test)]
mod test {
use super::*;
Expand Down
2 changes: 2 additions & 0 deletions canister/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ pub fn init(config: Config) {
with_state_mut(|s| s.syncing_state.syncing = config.syncing);
with_state_mut(|s| s.disable_api_if_not_fully_synced = config.disable_api_if_not_fully_synced);
with_state_mut(|s| s.watchdog_canister = config.watchdog_canister);
with_state_mut(|s| s.burn_cycles = config.burn_cycles);
with_state_mut(|s| s.fees = config.fees);
}

Expand Down Expand Up @@ -143,6 +144,7 @@ pub fn get_config() -> Config {
api_access: s.api_access,
disable_api_if_not_fully_synced: s.disable_api_if_not_fully_synced,
watchdog_canister: s.watchdog_canister,
burn_cycles: s.burn_cycles,
})
}

Expand Down
7 changes: 7 additions & 0 deletions canister/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ pub struct State {
/// The watchdog canister has the authority to disable the Bitcoin canister's API
/// if it suspects that there is a problem.
pub watchdog_canister: Option<Principal>,

/// If enabled, continuously burns all cycles in the canister's balance
/// to count towards the IC's burn rate.
/// NOTE: serde(default) is used here for backward-compatibility.
#[serde(default)]
pub burn_cycles: Flag,
}

impl State {
Expand All @@ -85,6 +91,7 @@ impl State {
api_access: Flag::Enabled,
disable_api_if_not_fully_synced: Flag::Enabled,
watchdog_canister: None,
burn_cycles: Flag::Disabled,
}
}

Expand Down
29 changes: 27 additions & 2 deletions canister/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{init, test_utils::random_p2pkh_address, Config};
use bitcoin::consensus::{Decodable, Encodable};
use bitcoin::{Block as BitcoinBlock, BlockHeader};
use byteorder::{LittleEndian, ReadBytesExt};
use ic_btc_interface::{GetUtxosResponse, Network, Txid, UtxosFilter};
use ic_btc_interface::{Flag, GetUtxosResponse, Network, Txid, UtxosFilter};
use ic_btc_interface::{OutPoint, Utxo};
use ic_btc_types::{Block, BlockHash};
use ic_cdk::api::call::RejectionCode;
Expand Down Expand Up @@ -719,7 +719,10 @@ async fn test_syncing_with_next_block_headers() {

#[async_std::test]
async fn cycles_burnt_are_tracked_in_metrics() {
crate::init(crate::Config::default());
crate::init(crate::Config {
burn_cycles: Flag::Enabled,
..Default::default()
});

let cycles_burnt_0 = crate::with_state(|state| state.metrics.cycles_burnt);

Expand Down Expand Up @@ -748,3 +751,25 @@ async fn cycles_burnt_are_tracked_in_metrics() {

assert_eq!(cycles_burnt_3, Some(3 * burn_amount));
}

#[async_std::test]
async fn cycles_are_not_burnt_when_flag_is_disabled() {
crate::init(crate::Config {
burn_cycles: Flag::Disabled,
..Default::default()
});

assert_eq!(
crate::with_state(|state| state.metrics.cycles_burnt),
Some(0)
);

// Run the heartbeat.
heartbeat().await;

// No cycles should be burnt.
assert_eq!(
crate::with_state(|state| state.metrics.cycles_burnt),
Some(0)
);
}
1 change: 1 addition & 0 deletions e2e-tests/charge-cycles-on-reject.sh
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ dfx deploy bitcoin --argument "(record {
api_access = variant { enabled };
disable_api_if_not_fully_synced = variant { enabled };
watchdog_canister = null;
burn_cycles = variant { enabled };
})"

check_charging()
Expand Down
1 change: 1 addition & 0 deletions e2e-tests/cycles_burn.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ dfx deploy --no-wallet --with-cycles "$INITIAL_BALANCE" bitcoin --argument "(rec
api_access = variant { enabled };
disable_api_if_not_fully_synced = variant { enabled };
watchdog_canister = null;
burn_cycles = variant { enabled };
})"

sleep 3
Expand Down
2 changes: 2 additions & 0 deletions e2e-tests/disable-api-if-not-fully-synced-flag.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ dfx deploy --no-wallet bitcoin --argument "(record {
api_access = variant { enabled };
disable_api_if_not_fully_synced = variant { enabled };
watchdog_canister = null;
burn_cycles = variant { enabled };
})"

# Wait until the ingestion of stable blocks is complete.
Expand Down Expand Up @@ -135,6 +136,7 @@ dfx deploy --no-wallet bitcoin --argument "(record {
api_access = variant { enabled };
disable_api_if_not_fully_synced = variant { disabled };
watchdog_canister = null;
burn_cycles = variant { enabled };
})"

# Wait until the ingestion of stable blocks is complete.
Expand Down
1 change: 1 addition & 0 deletions e2e-tests/scenario-1.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ dfx deploy --no-wallet bitcoin --argument "(record {
api_access = variant { enabled };
disable_api_if_not_fully_synced = variant { enabled };
watchdog_canister = null;
burn_cycles = variant { enabled };
})"

# Wait until the ingestion of stable blocks is complete.
Expand Down
1 change: 1 addition & 0 deletions e2e-tests/scenario-2.sh
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ dfx deploy --no-wallet bitcoin --argument "(record {
api_access = variant { enabled };
disable_api_if_not_fully_synced = variant { enabled };
watchdog_canister = null;
burn_cycles = variant { enabled };
})"

# Wait until the ingestion of stable blocks is complete.
Expand Down
1 change: 1 addition & 0 deletions e2e-tests/scenario-3.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ dfx deploy --no-wallet bitcoin --argument "(record {
api_access = variant { enabled };
disable_api_if_not_fully_synced = variant { enabled };
watchdog_canister = null;
burn_cycles = variant { enabled };
})"

# Send transaction valid transaction
Expand Down
1 change: 1 addition & 0 deletions e2e-tests/set_config.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ dfx deploy --no-wallet bitcoin --argument "(record {
api_access = variant { enabled };
disable_api_if_not_fully_synced = variant { enabled };
watchdog_canister = null;
burn_cycles = variant { enabled };
})"

# The stability threshold is zero
Expand Down
1 change: 1 addition & 0 deletions e2e-tests/upgradability.sh
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ ARGUMENT="(record {
api_access = variant { enabled };
disable_api_if_not_fully_synced = variant { enabled };
watchdog_canister = null;
burn_cycles = variant { enabled };
})"

# Run dfx stop if we run into errors and remove the downloaded wasm.
Expand Down
7 changes: 6 additions & 1 deletion interface/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -472,7 +472,7 @@ pub enum Flag {
Disabled,
}

/// The payload used to initialize the canister.
/// The config and payload used to initialize the canister.
#[derive(CandidType, Deserialize, Debug)]
pub struct Config {
pub stability_threshold: u128,
Expand All @@ -499,6 +499,10 @@ pub struct Config {
/// The watchdog canister has the authority to disable the Bitcoin canister's API
/// if it suspects that there is a problem.
pub watchdog_canister: Option<Principal>,

/// If enabled, continuously burns all cycles in its balance
/// (to count towards the IC's burn rate).
pub burn_cycles: Flag,
}

impl Default for Config {
Expand All @@ -512,6 +516,7 @@ impl Default for Config {
api_access: Flag::Enabled,
disable_api_if_not_fully_synced: Flag::Enabled,
watchdog_canister: None,
burn_cycles: Flag::Disabled,
}
}
}
Expand Down
Loading