Skip to content

Commit

Permalink
xcvm: change XCVMAck into an enum; and swap values (#3989)
Browse files Browse the repository at this point in the history
XCVMAck can hold limited set of values (success or failure) so rather
than writing it as a newtype around u8 change it into an enum.  This
simplifies any code that needs to match on an XCVMAck value since it
doesn’t need to check values greater than 1.

Furthermore, rename the variants to Ok and Fail to better fit Rust
naming and be more distinguishable; and swap the value so that Ok is
zero and Fail is one.


Required for merge:
- [x] `pr-workflow-check / draft-release-check` is ✅ success
- Other rules GitHub shows you, or can be read in
[configuration](../terraform/github.com/branches.tf)

Makes review faster:
- [x] PR title is my best effort to provide summary of changes and has
clear text to be part of release notes
- [x] I marked PR by `misc` label if it should not be in release notes
- [x] Linked Zenhub/Github/Slack/etc reference if one exists
- [x] I was clear on what type of deployment required to release my
changes (node, runtime, contract, indexer, on chain operation, frontend,
infrastructure) if any in PR title or description
- [x] Added reviewer into `Reviewers`
- [ ] I tagged(`@`) or used other form of notification of one person who
I think can handle best review of this PR
- [x] I have proved that PR has no general regressions of relevant
features and processes required to release into production
- [x] Any dependency updates made, was done according guides from
relevant dependency
- Clicking all checkboxes
- Adding detailed description of changes when it feels appropriate (for
example when PR is big)
  • Loading branch information
mina86 authored Jul 31, 2023
1 parent 8c251dd commit 02ac77a
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 29 deletions.
16 changes: 5 additions & 11 deletions code/xcvm/cosmwasm/contracts/gateway/src/contract/ibc/one.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,9 +112,10 @@ pub fn ibc_packet_receive(
Ok(SubMsg::reply_always(msg, EXEC_PROGRAM_REPLY_ID))
})();
Ok(match msg {
Ok(msg) => response.set_ack(XCVMAck::OK).add_submessage(msg),
Err(err) =>
response.add_event(make_ibc_failure_event(err.to_string())).set_ack(XCVMAck::KO),
Ok(msg) => response.set_ack(XCVMAck::Ok).add_submessage(msg),
Err(err) => response
.add_event(make_ibc_failure_event(err.to_string()))
.set_ack(XCVMAck::Fail),
})
}

Expand All @@ -123,14 +124,7 @@ pub fn ibc_packet_ack(_deps: DepsMut, _env: Env, msg: IbcPacketAckMsg) -> Result
let ack = XCVMAck::try_from(msg.acknowledgement.data.as_slice())
.map_err(|_| ContractError::InvalidAck)?;
let _: XcPacket = decode_packet(&msg.original_packet.data).map_err(ContractError::Protobuf)?;
match ack {
// https://github.com/cosmos/ibc/pull/998
XCVMAck::OK => (),
XCVMAck::KO => (),
_ => return Err(ContractError::InvalidAck),
};
Ok(IbcBasicResponse::default()
.add_event(make_event("ack").add_attribute("ack", ack.value().to_string())))
Ok(IbcBasicResponse::default().add_event(make_event("ack").add_attribute("ack", ack)))
}

#[cfg_attr(not(feature = "library"), cosmwasm_std::entry_point)]
Expand Down
4 changes: 2 additions & 2 deletions code/xcvm/cosmwasm/contracts/gateway/src/contract/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result<Response> {
fn handle_exec_reply(msg: Reply) -> Result {
let (data, event) = match msg.result {
SubMsgResult::Ok(_) =>
(XCVMAck::OK, make_event("receive").add_attribute("result", "success")),
SubMsgResult::Err(err) => (XCVMAck::KO, make_ibc_failure_event(err.to_string())),
(XCVMAck::Ok, make_event("receive").add_attribute("result", "success")),
SubMsgResult::Err(err) => (XCVMAck::Fail, make_ibc_failure_event(err.to_string())),
};
Ok(Response::default().add_event(event).set_data(data))
}
2 changes: 1 addition & 1 deletion code/xcvm/cosmwasm/tests/src/tests/suite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -769,7 +769,7 @@ mod cross_chain {

// The relayer must obtain a successful ack on the destination, and nothing on the source
// after relaying the ack itself.
assert_eq!(relay_data, vec![Some(XCVMAck::OK.into()), None]);
assert_eq!(relay_data, vec![Some(XCVMAck::Ok.into()), None]);

// We don't dispatch any information in the data field.
assert_eq!(dispatch_data, None);
Expand Down
45 changes: 30 additions & 15 deletions code/xcvm/lib/core/src/packet.rs
Original file line number Diff line number Diff line change
@@ -1,43 +1,58 @@
use crate::{Displayed, Funds, UserOrigin};
use alloc::{vec, vec::Vec};
use alloc::{string::String, vec::Vec};
use cosmwasm_std::Binary;
use parity_scale_codec::{Decode, Encode};
use scale_info::TypeInfo;
use serde::{Deserialize, Serialize};

#[derive(Debug, PartialEq, Eq)]
#[repr(transparent)]
pub struct XCVMAck(u8);
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum XCVMAck {
Ok,
Fail,
}

impl XCVMAck {
pub const KO: XCVMAck = XCVMAck(0);
pub const OK: XCVMAck = XCVMAck(1);
pub fn into_vec(self) -> Vec<u8> {
self.into()
fn into_byte(self) -> u8 {
match self {
Self::Ok => 0,
Self::Fail => 1,
}
}
pub fn value(self) -> u8 {
self.0

fn try_from_byte(value: u8) -> Result<Self, ()> {
match value {
0 => Ok(Self::Ok),
1 => Ok(Self::Fail),
_ => Err(()),
}
}
}

impl From<XCVMAck> for Binary {
fn from(value: XCVMAck) -> Self {
Binary::from(value.into_vec())
Binary::from(Vec::from(value))
}
}

impl From<XCVMAck> for Vec<u8> {
fn from(XCVMAck(x): XCVMAck) -> Self {
vec![x]
fn from(value: XCVMAck) -> Self {
[value.into_byte()].to_vec()
}
}

impl From<XCVMAck> for String {
fn from(ack: XCVMAck) -> Self {
let digit = [b'0' + ack.into_byte()];
// SAFETY: digit is always an ASCII digit
Self::from(unsafe { core::str::from_utf8_unchecked(&digit[..]) })
}
}

impl TryFrom<&[u8]> for XCVMAck {
type Error = ();
fn try_from(value: &[u8]) -> Result<Self, Self::Error> {
match value {
[0] => Ok(XCVMAck::KO),
[1] => Ok(XCVMAck::OK),
[byte] => Self::try_from_byte(*byte),
_ => Err(()),
}
}
Expand Down

0 comments on commit 02ac77a

Please sign in to comment.