From 66728df71ec9f558d2b4ef665746c2b6c7d67206 Mon Sep 17 00:00:00 2001 From: Donovan Solms Date: Wed, 11 Oct 2023 13:39:45 +0200 Subject: [PATCH 1/6] fix(controller_acks): Add handling of heartbeat acks --- contracts/controller/src/contract.rs | 6 +- contracts/controller/src/ibc.rs | 167 ++++++++++++---------- contracts/satellite_neutron/.cargo/config | 4 + 3 files changed, 99 insertions(+), 78 deletions(-) create mode 100644 contracts/satellite_neutron/.cargo/config diff --git a/contracts/controller/src/contract.rs b/contracts/controller/src/contract.rs index 6ba434d..42563db 100644 --- a/contracts/controller/src/contract.rs +++ b/contracts/controller/src/contract.rs @@ -200,7 +200,7 @@ mod tests { proposal_id, messages: vec![proposal_msg.clone()], }; - let resp = execute(deps.as_mut(), env.clone(), info, msg.clone()).unwrap(); + let resp = execute(deps.as_mut(), env.clone(), info, msg).unwrap(); assert_eq!(resp.messages.len(), 1); let real_timeout = IbcTimeout::with_timestamp(env.block.time.plus_seconds(360)); @@ -210,7 +210,7 @@ mod tests { timeout, data, }) if channel_id == channel_id && timeout == &real_timeout => { - let msg: SatelliteMsg = from_binary(&data).unwrap(); + let msg: SatelliteMsg = from_binary(data).unwrap(); assert_eq!( msg, SatelliteMsg::ExecuteProposal { @@ -223,7 +223,7 @@ mod tests { } let state = PROPOSAL_STATE - .load(deps.as_ref().storage, proposal_id.into()) + .load(deps.as_ref().storage, proposal_id) .unwrap(); assert_eq!(state, ProposalStatus::InProgress {}) } diff --git a/contracts/controller/src/ibc.rs b/contracts/controller/src/ibc.rs index 316ac73..ccf42fc 100644 --- a/contracts/controller/src/ibc.rs +++ b/contracts/controller/src/ibc.rs @@ -5,11 +5,10 @@ use cosmwasm_std::{ IbcReceiveResponse, StdError, StdResult, SubMsg, }; -use astro_satellite_package::IbcAckResult; -use ibc_controller_package::astroport_governance::assembly::ProposalStatus; - -use ibc_controller_package::astroport_governance::assembly::ExecuteMsg as AssemblyExecuteMsg; -use ibc_controller_package::IbcProposal; +use astro_satellite_package::{IbcAckResult, SatelliteMsg}; +use ibc_controller_package::astroport_governance::assembly::{ + ExecuteMsg as AssemblyExecuteMsg, ProposalStatus, +}; use crate::state::{CONFIG, LAST_ERROR, PROPOSAL_STATE}; @@ -99,33 +98,42 @@ pub fn ibc_packet_timeout( _env: Env, msg: IbcPacketTimeoutMsg, ) -> StdResult { - let ibc_proposal: IbcProposal = from_binary(&msg.packet.data)?; - let new_status = PROPOSAL_STATE.update(deps.storage, ibc_proposal.id, |state| match state { - None => Err(StdError::generic_err(format!( - "Proposal {} was not executed via controller", - ibc_proposal.id - ))), - Some(state) => { - if state == ProposalStatus::InProgress { - Ok(ProposalStatus::Failed {}) - } else { - Err(StdError::generic_err(format!( - "Proposal id: {} state is already {}", - ibc_proposal.id, state - ))) - } - } - })?; - let config = CONFIG.load(deps.storage)?; + let mut res = IbcBasicResponse::new(); + + let satellite_msg: SatelliteMsg = from_binary(&msg.packet.data)?; + match satellite_msg { + SatelliteMsg::ExecuteProposal { id, .. } => { + // The original packet was a proposal + let new_status = PROPOSAL_STATE.update(deps.storage, id, |state| match state { + None => Err(StdError::generic_err(format!( + "Proposal {} was not executed via controller", + id + ))), + Some(state) => { + if state == ProposalStatus::InProgress { + Ok(ProposalStatus::Failed {}) + } else { + Err(StdError::generic_err(format!( + "Proposal id: {} state is already {}", + id, state + ))) + } + } + })?; + let config = CONFIG.load(deps.storage)?; - Ok(IbcBasicResponse::new() - .add_submessage(confirm_assembly( - &config.owner, - ibc_proposal.id, - new_status, - )?) - .add_attribute("action", "packet_timeout") - .add_attribute("proposal_id", ibc_proposal.id.to_string())) + res = res + .add_submessage(confirm_assembly(&config.owner, id, new_status)?) + .add_attribute("action", "proposal_timeout") + .add_attribute("proposal_id", id.to_string()); + } + SatelliteMsg::Heartbeat {} => { + // The original packet was a heartbeat + // We don't do anything with the timeout for a heartbeat + res = res.add_attribute("action", "heartbeat_timeout") + } + } + Ok(res) } #[cfg_attr(not(feature = "library"), entry_point)] @@ -134,42 +142,51 @@ pub fn ibc_packet_ack( _env: Env, msg: IbcPacketAckMsg, ) -> StdResult { + let mut res = IbcBasicResponse::new(); + let ibc_ack: IbcAckResult = from_binary(&msg.acknowledgement.data)?; - let ibc_proposal: IbcProposal = from_binary(&msg.original_packet.data)?; - let mut err_msg = "".to_string(); - let new_status = PROPOSAL_STATE.update(deps.storage, ibc_proposal.id, |state| match state { - None => Err(StdError::generic_err(format!( - "Proposal {} was not executed via controller", - ibc_proposal.id - ))), - Some(state) => { - if state == ProposalStatus::InProgress { - match ibc_ack { - IbcAckResult::Ok(_) => Ok(ProposalStatus::Executed {}), - IbcAckResult::Error(err) => { - err_msg = err; - Ok(ProposalStatus::Failed {}) + let satellite_msg: SatelliteMsg = from_binary(&msg.original_packet.data)?; + match satellite_msg { + SatelliteMsg::ExecuteProposal { id, .. } => { + // The original packet was a proposal + let mut err_msg = "".to_string(); + let new_status = PROPOSAL_STATE.update(deps.storage, id, |state| match state { + None => Err(StdError::generic_err(format!( + "Proposal {} was not executed via controller", + id + ))), + Some(state) => { + if state == ProposalStatus::InProgress { + match ibc_ack { + IbcAckResult::Ok(_) => Ok(ProposalStatus::Executed {}), + IbcAckResult::Error(err) => { + err_msg = err; + Ok(ProposalStatus::Failed {}) + } + } + } else { + Err(StdError::generic_err(format!( + "Proposal id: {} state is already {}", + id, state + ))) } } - } else { - Err(StdError::generic_err(format!( - "Proposal id: {} state is already {}", - ibc_proposal.id, state - ))) - } + })?; + LAST_ERROR.save(deps.storage, &err_msg)?; + let config = CONFIG.load(deps.storage)?; + + res = res + .add_submessage(confirm_assembly(&config.owner, id, new_status)?) + .add_attribute("action", "proposal_ack") + .add_attribute("proposal_id", id.to_string()); } - })?; - LAST_ERROR.save(deps.storage, &err_msg)?; - let config = CONFIG.load(deps.storage)?; - - Ok(IbcBasicResponse::new() - .add_submessage(confirm_assembly( - &config.owner, - ibc_proposal.id, - new_status, - )?) - .add_attribute("action", "packet_ack") - .add_attribute("proposal_id", ibc_proposal.id.to_string())) + SatelliteMsg::Heartbeat {} => { + // The original packet was a heartbeat + // We don't do anything with the ack from a heartbeat + res = res.add_attribute("action", "heartbeat_ack") + } + } + Ok(res) } #[cfg_attr(not(feature = "library"), entry_point)] @@ -217,7 +234,7 @@ mod tests { // Ok acknowledgment let ack_msg = mock_ibc_packet_ack( channel_id, - &IbcProposal { + &SatelliteMsg::ExecuteProposal { id: proposal_id, messages: vec![], }, @@ -230,7 +247,7 @@ mod tests { .attributes .contains(&attr("proposal_id", proposal_id.to_string()))); let state = PROPOSAL_STATE - .load(deps.as_ref().storage, proposal_id.into()) + .load(deps.as_ref().storage, proposal_id) .unwrap(); assert_eq!(state, ProposalStatus::Executed); @@ -254,7 +271,7 @@ mod tests { execute(deps.as_mut(), env.clone(), info, msg).unwrap(); let ack_msg = mock_ibc_packet_ack( channel_id, - &IbcProposal { + &SatelliteMsg::ExecuteProposal { id: proposal_id, messages: vec![], }, @@ -268,7 +285,7 @@ mod tests { .attributes .contains(&attr("proposal_id", proposal_id.to_string()))); let state = PROPOSAL_STATE - .load(deps.as_ref().storage, proposal_id.into()) + .load(deps.as_ref().storage, proposal_id) .unwrap(); assert_eq!(state, ProposalStatus::Failed); @@ -288,14 +305,14 @@ mod tests { // Previous proposal state was not changed let state = PROPOSAL_STATE - .load(deps.as_ref().storage, (proposal_id - 1).into()) + .load(deps.as_ref().storage, proposal_id - 1) .unwrap(); assert_eq!(state, ProposalStatus::Executed); // Proposal with unknown id let ack_msg = mock_ibc_packet_ack( channel_id, - &IbcProposal { + &SatelliteMsg::ExecuteProposal { id: 128, messages: vec![], }, @@ -322,8 +339,8 @@ mod tests { execute(deps.as_mut(), env.clone(), info, msg).unwrap(); let timeout_msg = mock_ibc_packet_timeout( - &channel_id, - &IbcProposal { + channel_id, + &SatelliteMsg::ExecuteProposal { id: proposal_id, messages: vec![], }, @@ -335,7 +352,7 @@ mod tests { .contains(&attr("proposal_id", proposal_id.to_string()))); let state = PROPOSAL_STATE - .load(deps.as_ref().storage, proposal_id.into()) + .load(deps.as_ref().storage, proposal_id) .unwrap(); assert_eq!(state, ProposalStatus::Failed); @@ -366,14 +383,14 @@ mod tests { // timeout msg with unknown proposal id will fail let timeout_msg = mock_ibc_packet_timeout( - &channel_id, - &IbcProposal { + channel_id, + &SatelliteMsg::ExecuteProposal { id: 128, messages: vec![], }, ) .unwrap(); - let err = ibc_packet_timeout(deps.as_mut(), env.clone(), timeout_msg).unwrap_err(); + let err = ibc_packet_timeout(deps.as_mut(), env, timeout_msg).unwrap_err(); assert_eq!( err, StdError::generic_err("Proposal 128 was not executed via controller") diff --git a/contracts/satellite_neutron/.cargo/config b/contracts/satellite_neutron/.cargo/config new file mode 100644 index 0000000..8438b9b --- /dev/null +++ b/contracts/satellite_neutron/.cargo/config @@ -0,0 +1,4 @@ +[alias] +wasm = "build --release --lib --target wasm32-unknown-unknown" +unit-test = "test --lib" +schema = "run --example satellite_neutron_schema" \ No newline at end of file From 111de799016888356b8e6743b698fad86e2e9500 Mon Sep 17 00:00:00 2001 From: Donovan Solms Date: Wed, 11 Oct 2023 13:44:39 +0200 Subject: [PATCH 2/6] fix(controller-acks): Bump contract version to 1.0.1 --- Cargo.lock | 2 +- contracts/controller/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 68ce617..ae05900 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -720,7 +720,7 @@ dependencies = [ [[package]] name = "ibc-controller" -version = "1.0.0" +version = "1.0.1" dependencies = [ "astro-satellite-package", "astroport-ibc", diff --git a/contracts/controller/Cargo.toml b/contracts/controller/Cargo.toml index 886fd5f..4b1148b 100644 --- a/contracts/controller/Cargo.toml +++ b/contracts/controller/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ibc-controller" -version = "1.0.0" +version = "1.0.1" authors = ["Astroport"] license = "Apache-2.0" description = "IBC controller contract intended to be hosted on the main chain." From 5e050488c772fa587f16cf8bb655496e33846494 Mon Sep 17 00:00:00 2001 From: Donovan Solms Date: Wed, 11 Oct 2023 14:25:59 +0200 Subject: [PATCH 3/6] fix(cosmwasm-check): Fix failing tests for CosmWasm check --- .github/workflows/check_artifacts.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/check_artifacts.yml b/.github/workflows/check_artifacts.yml index db0f0d3..e68d895 100644 --- a/.github/workflows/check_artifacts.yml +++ b/.github/workflows/check_artifacts.yml @@ -68,7 +68,7 @@ jobs: fail-on-cache-miss: true - name: Install cosmwasm-check # Uses --debug for compilation speed - run: cargo install --debug --version 1.3.1 cosmwasm-check + run: cargo install --debug --locked --version 1.3.1 cosmwasm-check - name: Cosmwasm check run: | cosmwasm-check $GITHUB_WORKSPACE/artifacts/*.wasm --available-capabilities staking,cosmwasm_1_1,neutron,iterator,stargate From 4b81661fb223b801c929f576818e7b7821ef59de Mon Sep 17 00:00:00 2001 From: Donovan Solms Date: Thu, 12 Oct 2023 13:36:45 +0200 Subject: [PATCH 4/6] Bump IBC crate to v1.2.1 --- Cargo.lock | 2 +- packages/astroport-ibc/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index ae05900..2054ccf 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -98,7 +98,7 @@ dependencies = [ [[package]] name = "astroport-ibc" -version = "1.2.0" +version = "1.2.1" dependencies = [ "cosmwasm-schema", ] diff --git a/packages/astroport-ibc/Cargo.toml b/packages/astroport-ibc/Cargo.toml index 503a340..3b89b3a 100644 --- a/packages/astroport-ibc/Cargo.toml +++ b/packages/astroport-ibc/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "astroport-ibc" -version = "1.2.0" +version = "1.2.1" edition = "2021" # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html From 27570ec863b913bce7d16bb9e4a9ba635d281240 Mon Sep 17 00:00:00 2001 From: Donovan Solms Date: Thu, 12 Oct 2023 14:04:13 +0200 Subject: [PATCH 5/6] fix(tests): Add heartbeat to tests --- contracts/controller/src/ibc.rs | 61 ++++++++++++++++++++++++++++++--- 1 file changed, 57 insertions(+), 4 deletions(-) diff --git a/contracts/controller/src/ibc.rs b/contracts/controller/src/ibc.rs index ccf42fc..fc863ab 100644 --- a/contracts/controller/src/ibc.rs +++ b/contracts/controller/src/ibc.rs @@ -130,7 +130,9 @@ pub fn ibc_packet_timeout( SatelliteMsg::Heartbeat {} => { // The original packet was a heartbeat // We don't do anything with the timeout for a heartbeat - res = res.add_attribute("action", "heartbeat_timeout") + res = res + .add_attribute("action", "heartbeat_timeout") + .add_attribute("channel_id", msg.packet.src.channel_id) } } Ok(res) @@ -183,7 +185,9 @@ pub fn ibc_packet_ack( SatelliteMsg::Heartbeat {} => { // The original packet was a heartbeat // We don't do anything with the ack from a heartbeat - res = res.add_attribute("action", "heartbeat_ack") + res = res + .add_attribute("action", "heartbeat_ack") + .add_attribute("channel_id", msg.original_packet.src.channel_id) } } Ok(res) @@ -220,8 +224,14 @@ mod tests { } } + fn mock_ibc_heartbeat(channel_id: &str) -> ExecuteMsg { + ExecuteMsg::SendHeartbeat { + channels: vec![channel_id.to_string()], + } + } + #[test] - fn channel_ack() { + fn channel_proposal_ack() { let (mut deps, env, info) = mock_all(OWNER); init_contract(&mut deps, env.clone(), info.clone()); @@ -328,7 +338,32 @@ mod tests { } #[test] - fn channel_timeout() { + fn channel_heartbeat_ack() { + let (mut deps, env, info) = mock_all(OWNER); + init_contract(&mut deps, env.clone(), info.clone()); + + let channel_id = "channel-0"; + + let msg = mock_ibc_heartbeat(channel_id); + execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + + // Ok acknowledgment + let ack_msg = mock_ibc_packet_ack( + channel_id, + &SatelliteMsg::Heartbeat {}, + IbcAcknowledgement::encode_json(&IbcAckResult::Ok(Binary::default())).unwrap(), + ) + .unwrap(); + let resp = ibc_packet_ack(deps.as_mut(), env, ack_msg).unwrap(); + + assert!(resp + .attributes + .contains(&attr("action", "heartbeat_ack".to_string()))); + assert!(resp.attributes.contains(&attr("channel_id", channel_id))); + } + + #[test] + fn channel_proposal_timeout() { let (mut deps, env, info) = mock_all(OWNER); init_contract(&mut deps, env.clone(), info.clone()); @@ -397,6 +432,24 @@ mod tests { ) } + #[test] + fn channel_heartbeat_timeout() { + let (mut deps, env, info) = mock_all(OWNER); + init_contract(&mut deps, env.clone(), info.clone()); + + let channel_id = "channel-0"; + + let msg = mock_ibc_heartbeat(channel_id); + execute(deps.as_mut(), env.clone(), info, msg).unwrap(); + + let timeout_msg = mock_ibc_packet_timeout(channel_id, &SatelliteMsg::Heartbeat {}).unwrap(); + let resp = ibc_packet_timeout(deps.as_mut(), env, timeout_msg).unwrap(); + assert!(resp + .attributes + .contains(&attr("action", "heartbeat_timeout".to_string()))); + assert!(resp.attributes.contains(&attr("channel_id", channel_id))); + } + #[test] fn channel_close() { let close_msg = From 7d79fbee4f6e62eaa32a22b3810dff4bed84564f Mon Sep 17 00:00:00 2001 From: Donovan Solms Date: Thu, 12 Oct 2023 14:29:07 +0200 Subject: [PATCH 6/6] fix(version): Bump controller version --- Cargo.lock | 2 +- contracts/controller/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2054ccf..3cc7eba 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -720,7 +720,7 @@ dependencies = [ [[package]] name = "ibc-controller" -version = "1.0.1" +version = "1.1.0" dependencies = [ "astro-satellite-package", "astroport-ibc", diff --git a/contracts/controller/Cargo.toml b/contracts/controller/Cargo.toml index 4b1148b..5605556 100644 --- a/contracts/controller/Cargo.toml +++ b/contracts/controller/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "ibc-controller" -version = "1.0.1" +version = "1.1.0" authors = ["Astroport"] license = "Apache-2.0" description = "IBC controller contract intended to be hosted on the main chain."