Skip to content

Commit

Permalink
feat(RUN-905): Canister can use itself as store for chunked install
Browse files Browse the repository at this point in the history
  • Loading branch information
adambratschikaye committed Feb 7, 2024
1 parent 6b7b236 commit 895ee8b
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 1 deletion.
65 changes: 65 additions & 0 deletions rs/execution_environment/src/execution/install_code/tests.rs
Expand Up @@ -1900,6 +1900,71 @@ fn install_chunked_fails_from_noncontroller_of_store() {
}
}

/// A canister should be able to use itself as the `store_canister` in
/// `install_chunked_code` even if it isn't its own controller.
#[test]
fn install_chunked_succeeds_from_store_canister() {
const CYCLES: Cycles = Cycles::new(1_000_000_000_000_000);

let mut test = ExecutionTestBuilder::new()
.with_wasm_chunk_store(FlagStatus::Enabled)
.build();

let target_canister = test.create_canister(CYCLES);
// Install universal canister to canister which will also be the store and
// make it a controller of the target.
let store_canister = test
.canister_from_cycles_and_binary(CYCLES, UNIVERSAL_CANISTER_WASM.into())
.unwrap();
test.set_controller(target_canister, store_canister.into())
.unwrap();

// Upload universal canister chunk to store.
let uc_wasm = UNIVERSAL_CANISTER_WASM;
let hash = UploadChunkReply::decode(&get_reply(
test.subnet_message(
"upload_chunk",
UploadChunkArgs {
canister_id: store_canister.into(),
chunk: uc_wasm.to_vec(),
}
.encode(),
),
))
.unwrap()
.hash;

// Install UC wasm on target canister from store canister should succeed
// even though the store canister isn't its own controller.
assert!(!test
.canister_state(store_canister)
.system_state
.controllers
.contains(&store_canister.get()));

let install = wasm()
.call_with_cycles(
CanisterId::ic_00(),
Method::InstallChunkedCode,
call_args()
.other_side(
InstallChunkedCodeArgs::new(
CanisterInstallModeV2::Install,
target_canister,
Some(store_canister),
vec![hash.clone()],
hash,
vec![],
)
.encode(),
)
.on_reject(wasm().reject_message().reject()),
Cycles::new(CYCLES.get() / 2),
)
.build();
get_reply(test.ingress(store_canister, "update", install));
}

#[test]
fn install_with_dts_correctly_updates_system_state() {
let mut test = ExecutionTestBuilder::new()
Expand Down
6 changes: 5 additions & 1 deletion rs/execution_environment/src/execution_environment.rs
Expand Up @@ -2323,7 +2323,11 @@ impl ExecutionEnvironment {
format!("InstallChunkedCode Error: Store canister {} was not found on subnet {} of target canister {}", store_canister_id, state.metadata.own_subnet_id, args.target_canister_id()),
)
})?;
validate_controller(store_canister, &origin.origin())?;
// If the `store_canister` is different from the caller, we need
// to verify that the caller is a controller of the store.
if store_canister.canister_id().get() != origin.origin() {
validate_controller(store_canister, &origin.origin())?;
}
InstallCodeContext::chunked_install(
origin,
args,
Expand Down

2 comments on commit 895ee8b

@ilbertt
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adambratschikaye how does this change implies that a canister can use itself as the store canister?

@hpeebles
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It allows the canister to call install_chunked_code and use itself as the chunk store without having to be a controller of itself.
(Its a change I asked for!)

Please sign in to comment.