Skip to content

Commit

Permalink
Merge branch 'mraszyk/state-machine-submit-ingress' into 'master'
Browse files Browse the repository at this point in the history
fix(PocketIC): canister inspect message errors

This MR makes canister inspect message errors when executing ingress messages in PocketIC returned as canister execution results rather than request errors. This way, the function `update_call` in the PocketIC client library can handle this error without panicking. 

See merge request dfinity-lab/public/ic!17034
  • Loading branch information
mraszyk committed Jan 9, 2024
2 parents 2707a6d + fbb2909 commit d43bd00
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 6 deletions.
2 changes: 2 additions & 0 deletions rs/pocket_ic_server/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Changed
- Canisters are created with 100T cycles by default when using the provisional management canister API.

### Fixed
- Canister inspect message errors when executing ingress messages are returned as canister execution results rather than request errors.


## 2.0.1 - 2023-11-23
Expand Down
8 changes: 6 additions & 2 deletions rs/pocket_ic_server/src/pocket_ic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use ic_registry_routing_table::{CanisterIdRange, RoutingTable, CANISTER_IDS_PER_
use ic_registry_subnet_type::SubnetType;
use ic_state_machine_tests::{
EcdsaCurve, EcdsaKeyId, IngressState, IngressStatus, StateMachine, StateMachineBuilder,
StateMachineConfig, Time,
StateMachineConfig, SubmitIngressError, Time,
};
use ic_test_utilities::types::ids::subnet_test_id;
use ic_types::{CanisterId, PrincipalId, SubnetId};
Expand Down Expand Up @@ -478,10 +478,14 @@ impl Operation for ExecuteIngressMessage {
self.0.method,
self.0.payload,
) {
Err(e) => {
Err(SubmitIngressError::HttpError(e)) => {
eprintln!("Failed to submit ingress message: {}", e);
OpOut::Error(PocketIcError::BadIngressMessage(e))
}
Err(ic_state_machine_tests::SubmitIngressError::UserError(e)) => {
eprintln!("Failed to submit ingress message: {:?}", e);
Err::<ic_state_machine_tests::WasmResult, ic_state_machine_tests::UserError>(e).into()
}
Ok(msg_id) => {
// Now, we execute on all subnets until we have the result
let max_rounds = 100;
Expand Down
15 changes: 11 additions & 4 deletions rs/state_machine_tests/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ use ic_xnet_payload_builder::{
ExpectedIndices, RefillTaskHandle, XNetPayloadBuilderImpl, XNetPayloadBuilderMetrics,
XNetSlicePool,
};
use serde::Deserialize;

pub use ic_error_types::RejectCode;
use maplit::btreemap;
Expand All @@ -150,6 +151,12 @@ use tokio::sync::mpsc;
#[cfg(test)]
mod tests;

#[derive(Debug, Serialize, Clone, PartialEq, Eq, PartialOrd, Ord, Deserialize)]
pub enum SubmitIngressError {
HttpError(String),
UserError(UserError),
}

struct FakeVerifier;

impl Verifier for FakeVerifier {
Expand Down Expand Up @@ -1307,7 +1314,7 @@ impl StateMachine {
canister_id: CanisterId,
method: impl ToString,
payload: Vec<u8>,
) -> Result<MessageId, String> {
) -> Result<MessageId, SubmitIngressError> {
// Build `SignedIngress` with maximum ingress expiry and unique nonce,
// omitting delegations and signatures.
let ingress_expiry = (self.get_time() + MAX_INGRESS_TTL).as_nanos_since_unix_epoch();
Expand Down Expand Up @@ -1359,18 +1366,18 @@ impl StateMachine {

// Validate the size of the ingress message.
if msg.count_bytes() > ingress_registry_settings.max_ingress_bytes_per_message {
return Err(format!(
return Err(SubmitIngressError::HttpError(format!(
"Request {} is too large. Message byte size {} is larger than the max allowed {}.",
msg.id(),
msg.count_bytes(),
ingress_registry_settings.max_ingress_bytes_per_message
));
)));
}

// Run `IngressFilter` on the ingress message.
self.ingress_filter
.should_accept_ingress_message(state, &provisional_whitelist, msg.content())
.map_err(|e| e.to_string())?;
.map_err(|e| SubmitIngressError::UserError(e))?;

// All checks were successful at this point so we can push the ingress message to the ingress pool.
let message_id = msg.id();
Expand Down

0 comments on commit d43bd00

Please sign in to comment.