Skip to content

Commit

Permalink
Merge pull request #34 from astroport-fi/fix/controller-acks
Browse files Browse the repository at this point in the history 
fix(controller_acks): Add handling of heartbeat acks and timeouts
  • Loading branch information
@donovansolms
donovansolms authored Oct 12, 2023
2 parents ffb48eb + 7d79fbe commit 61f3cf9
Show file tree
Hide file tree
Showing 7 changed files with 159 additions and 85 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/check_artifacts.yml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 2 additions & 2 deletions Cargo.lock
View file

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

2 changes: 1 addition & 1 deletion contracts/controller/Cargo.toml
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "ibc-controller"
version = "1.0.0"
version = "1.1.0"
authors = ["Astroport"]
license = "Apache-2.0"
description = "IBC controller contract intended to be hosted on the main chain."
Expand Down
6 changes: 3 additions & 3 deletions contracts/controller/src/contract.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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 {
Expand All @@ -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 {})
}
Expand Down
224 changes: 147 additions & 77 deletions contracts/controller/src/ibc.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -99,33 +98,44 @@ pub fn ibc_packet_timeout(
_env: Env,
msg: IbcPacketTimeoutMsg,
) -> StdResult<IbcBasicResponse> {
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")
.add_attribute("channel_id", msg.packet.src.channel_id)
}
}
Ok(res)
}

#[cfg_attr(not(feature = "library"), entry_point)]
Expand All @@ -134,42 +144,53 @@ pub fn ibc_packet_ack(
_env: Env,
msg: IbcPacketAckMsg,
) -> StdResult<IbcBasicResponse> {
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")
.add_attribute("channel_id", msg.original_packet.src.channel_id)
}
}
Ok(res)
}

#[cfg_attr(not(feature = "library"), entry_point)]
Expand Down Expand Up @@ -203,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());

Expand All @@ -217,7 +244,7 @@ mod tests {
// Ok acknowledgment
let ack_msg = mock_ibc_packet_ack(
channel_id,
&IbcProposal {
&SatelliteMsg::ExecuteProposal {
id: proposal_id,
messages: vec![],
},
Expand All @@ -230,7 +257,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);

Expand All @@ -254,7 +281,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![],
},
Expand All @@ -268,7 +295,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);

Expand All @@ -288,14 +315,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![],
},
Expand All @@ -311,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());

Expand All @@ -322,8 +374,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![],
},
Expand All @@ -335,7 +387,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);

Expand Down Expand Up @@ -366,20 +418,38 @@ 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")
)
}

#[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 =
Expand Down
Loading

0 comments on commit 61f3cf9

Please sign in to comment.