Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Pass full data to callback #19

Closed
wants to merge 8 commits into from
Closed
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Cargo.lock

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

9 changes: 5 additions & 4 deletions contracts/note/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use cosmwasm_std::{
to_binary, Binary, Deps, DepsMut, Env, IbcMsg, IbcTimeout, MessageInfo, Response, StdResult,
};
use cw2::set_contract_version;
use polytone::callback::RequestType;
use polytone::{callback, ibc};

use crate::error::ContractError;
Expand Down Expand Up @@ -42,20 +43,20 @@ pub fn execute(
info: MessageInfo,
msg: ExecuteMsg,
) -> Result<Response, ContractError> {
let (msg, callback, timeout_seconds) = match msg {
let (msg, callback, timeout_seconds, request_type) = match msg {
ExecuteMsg::Execute {
msgs,
callback,
timeout_seconds,
} => (ibc::Msg::Execute { msgs }, callback, timeout_seconds),
} => (ibc::Msg::Execute { msgs }, callback, timeout_seconds, RequestType::Execute),
ExecuteMsg::Query {
msgs,
callback,
timeout_seconds,
} => (ibc::Msg::Query { msgs }, Some(callback), timeout_seconds),
} => (ibc::Msg::Query { msgs }, Some(callback), timeout_seconds, RequestType::Query),
};

callback::request_callback(deps.storage, deps.api, info.sender.clone(), callback)?;
callback::request_callback(deps.storage, deps.api, info.sender.clone(), callback, request_type)?;

let channel_id = CHANNEL
.may_load(deps.storage)?
Expand Down
1 change: 1 addition & 0 deletions contracts/proxy/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ cw-storage-plus = { workspace = true }
cw-utils = { workspace = true }
cw2 = { workspace = true }
thiserror = { workspace = true }
polytone = { workspace = true }

[dev-dependencies]
cw-multi-test = { workspace = true }
16 changes: 9 additions & 7 deletions contracts/proxy/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,10 @@
use cosmwasm_std::entry_point;
use cosmwasm_std::{
to_binary, Binary, Deps, DepsMut, Env, MessageInfo, Reply, Response, StdResult, SubMsg,
SubMsgResult,
SubMsgResponse, SubMsgResult,
};
use cw2::set_contract_version;
use cw_utils::parse_execute_response_data;
use polytone::ack::ack_success_execute;

use crate::error::ContractError;
use crate::msg::{ExecuteMsg, InstantiateMsg, QueryMsg};
Expand Down Expand Up @@ -69,16 +69,18 @@ pub fn reply(deps: DepsMut, _env: Env, msg: Reply) -> Result<Response, ContractE
match msg.result {
SubMsgResult::Err(error) => Err(ContractError::ExecutionFailure { idx: msg.id, error }),
SubMsgResult::Ok(res) => {
collector[msg.id as usize] = match res.data {
Some(data) => parse_execute_response_data(&data.0)?.data,
None => None,
};
collector[msg.id as usize] = Some(res);

if msg.id + 1 == collector.len() as u64 {
COLLECTOR.remove(deps.storage);
// Unwrap the options as we set it to Some
let collector = collector
.into_iter()
.map(|res| res.unwrap())
.collect::<Vec<SubMsgResponse>>();
Ok(Response::default()
.add_attribute("callbacks_processed", (msg.id + 1).to_string())
.set_data(to_binary(&collector)?))
.set_data(ack_success_execute(collector)))
} else {
COLLECTOR.save(deps.storage, &collector)?;
Ok(Response::default())
Expand Down
4 changes: 2 additions & 2 deletions contracts/proxy/src/state.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
use cosmwasm_std::{Addr, Binary};
use cosmwasm_std::{Addr, SubMsgResponse};
use cw_storage_plus::Item;

/// Stores the instantiator of the contract.
pub const INSTANTIATOR: Item<Addr> = Item::new("owner");

/// Stores a list of callback's currently being collected. Has no
/// value if none are being collected.
pub const COLLECTOR: Item<Vec<Option<Binary>>> = Item::new("callbacks");
pub const COLLECTOR: Item<Vec<Option<SubMsgResponse>>> = Item::new("callbacks");
3 changes: 2 additions & 1 deletion contracts/voice/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use cosmwasm_std::{
};
use cw2::set_contract_version;

use polytone::ack::ack_success_query;
use polytone::ibc::{Msg, Packet};

use crate::error::ContractError;
Expand Down Expand Up @@ -67,7 +68,7 @@ pub fn execute(
}
Ok(Response::default()
.add_attribute("method", "rx_query")
.set_data(to_binary(&results)?))
.set_data(ack_success_query(results)))
}
Msg::Execute { msgs } => {
let (instantiate, proxy) = if let Some(proxy) = SENDER_TO_PROXY.may_load(
Expand Down
13 changes: 5 additions & 8 deletions contracts/voice/src/ibc.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,14 @@
#[cfg(not(feature = "library"))]
use cosmwasm_std::entry_point;
use cosmwasm_std::{
from_binary, to_binary, Binary, DepsMut, Env, IbcBasicResponse, IbcChannelCloseMsg,
from_binary, to_binary, DepsMut, Env, IbcBasicResponse, IbcChannelCloseMsg,
IbcChannelConnectMsg, IbcChannelOpenMsg, IbcChannelOpenResponse, IbcPacketAckMsg,
IbcPacketReceiveMsg, IbcPacketTimeoutMsg, IbcReceiveResponse, Never, Reply, Response, SubMsg,
SubMsgResult, WasmMsg,
};

use cw_utils::{parse_reply_execute_data, MsgExecuteContractResponse};
use polytone::{
ack::{ack_fail, ack_success},
ibc::validate_order_and_version,
};
use polytone::{ack::ack_fail, callback::Callback, ibc::validate_order_and_version};

use crate::{error::ContractError, msg::ExecuteMsg, state::CHANNEL_TO_CONNECTION};

Expand Down Expand Up @@ -97,12 +94,12 @@ pub fn reply(_deps: DepsMut, _env: Env, msg: Reply) -> Result<Response, Contract
.add_attribute("ack_error", &e)
.set_data(ack_fail(e)),
SubMsgResult::Ok(_) => {
let data = parse_reply_execute_data(msg)
let data = parse_reply_execute_data(msg.clone())
.expect("execution succeded")
.data
.expect("proxy should set data");
match from_binary::<Vec<Option<Binary>>>(&data) {
Ok(d) => Response::default().set_data(ack_success(d)),
match from_binary::<Callback>(&data) {
Ok(_) => Response::default().set_data(data),
Err(e) => Response::default()
.set_data(ack_fail(format!("unmarshaling callback data: ({e})"))),
}
Expand Down
31 changes: 25 additions & 6 deletions packages/polytone/src/ack.rs
Original file line number Diff line number Diff line change
@@ -1,17 +1,27 @@
use cosmwasm_std::{from_binary, to_binary, Binary, IbcAcknowledgement};
use cosmwasm_schema::cw_serde;
use cosmwasm_std::{from_binary, to_binary, Binary, IbcAcknowledgement, SubMsgResponse};

pub use crate::callback::Callback;
use crate::callback::{CallbackData, RequestType};

pub type Ack = Callback;

#[cw_serde]
pub struct InterlError(String);
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There are some errors where we don't know what the type of the callback is, so created this type to match against before executing the callback

Copy link
Contributor

Choose a reason for hiding this comment

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

two thoughts about this type:

  1. i don't think this needs to be public as you take it back apart in unmarshal_ack?
  2. spelling:InternalError

also, a small test for you:

#[cfg(test)]
mod tests {
    use super::*;

    #[test]
    fn test_forge_success() {
        let err = "{\"execute\":{\"success\":[]}}".to_string();
        let ack = ack_fail(err.clone());

        // ack is now base64 of `"{\"execute\":{\"success\":[]}}"`
        // whereas a real ack looks like
        // `{"execute":{"success":[]}}`. note the string escaping and
        // opening/closing quotes.
        let result = unmarshal_ack(&IbcAcknowledgement::new(ack), RequestType::Execute);
        assert_eq!(result, Ack::Execute(CallbackData::Error(err)))
    }
}


/// Serializes an ACK-SUCCESS containing the provided data.
pub fn ack_success_query(c: Vec<Binary>) -> Binary {
to_binary(&Callback::Query(CallbackData::Success(c))).unwrap()
}

/// Serializes an ACK-SUCCESS containing the provided data.
pub fn ack_success(c: Vec<Option<Binary>>) -> Binary {
to_binary(&Callback::Success(c)).unwrap()
pub fn ack_success_execute(c: Vec<SubMsgResponse>) -> Binary {
to_binary(&Callback::Execute(CallbackData::Success(c))).unwrap()
}

/// Serializes an ACK-FAIL containing the provided error.
pub fn ack_fail(err: String) -> Binary {
to_binary(&Callback::Error(err)).unwrap()
to_binary(&InterlError(err)).unwrap()
}

/// Unmarshals an ACK from an acknowledgement returned by the SDK. If
Expand All @@ -38,6 +48,15 @@ pub fn ack_fail(err: String) -> Binary {
/// For an example of this, see this integration test:
///
/// <https://github.com/public-awesome/ics721/blob/3af19e421a95aec5291a0cabbe796c58698ac97f/e2e/adversarial_test.go#L274-L285>
pub fn unmarshal_ack(ack: &IbcAcknowledgement) -> Ack {
from_binary(&ack.data).unwrap_or_else(|_| Callback::Error(ack.data.to_base64()))
pub fn unmarshal_ack(ack: &IbcAcknowledgement, request_type: RequestType) -> Ack {
Art3miX marked this conversation as resolved.
Show resolved Hide resolved
if let Ok(err) = from_binary::<InterlError>(&ack.data) {
return match request_type {
RequestType::Execute => Callback::Execute(CallbackData::Error(err.0)),
RequestType::Query => Callback::Query(CallbackData::Error(err.0)),
};
}
from_binary(&ack.data).unwrap_or_else(|_| match request_type {
RequestType::Execute => Callback::Execute(CallbackData::Error(ack.data.to_base64())),
RequestType::Query => Callback::Query(CallbackData::Error(ack.data.to_base64())),
})
}
32 changes: 25 additions & 7 deletions packages/polytone/src/callback.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use cosmwasm_schema::cw_serde;
use cosmwasm_std::{
to_binary, Addr, Api, Binary, CosmosMsg, IbcPacketAckMsg, IbcPacketTimeoutMsg, StdResult,
Storage, WasmMsg,
Storage, SubMsgResponse, WasmMsg,
};
use cw_storage_plus::{Item, Map};

Expand Down Expand Up @@ -30,9 +30,17 @@ pub struct CallbackMessage {

#[cw_serde]
pub enum Callback {
/// Callback data from a query.
Query(CallbackData<Vec<Binary>>),
/// Callback data from message execution.
Execute(CallbackData<Vec<SubMsgResponse>>),
}

#[cw_serde]
pub enum CallbackData<T> {
/// Data returned from the host chain. Index n corresponds to the
/// result of executing the nth message/query.
Success(Vec<Option<Binary>>),
Success(T),
/// The first error that occured while executing the requested
/// messages/queries.
Error(String),
Expand Down Expand Up @@ -63,6 +71,7 @@ pub fn request_callback(
api: &dyn Api,
initiator: Addr,
request: Option<CallbackRequest>,
request_type: RequestType,
) -> StdResult<()> {
let seq = SEQ.may_load(storage)?.unwrap_or_default() + 1;
SEQ.save(storage, &seq)?;
Expand All @@ -78,6 +87,7 @@ pub fn request_callback(
initiator,
initiator_msg,
receiver,
request_type,
},
)?;
}
Expand Down Expand Up @@ -119,7 +129,7 @@ pub fn on_ack(
return None
};
CALLBACKS.remove(storage, original_packet.sequence);
let result = unmarshal_ack(acknowledgement);
let result = unmarshal_ack(acknowledgement, request.request_type.clone());
Some(callback_msg(request, result))
}

Expand All @@ -133,17 +143,25 @@ pub fn on_timeout(
return None
};
CALLBACKS.remove(storage, packet.sequence);
Some(callback_msg(
request,
Callback::Error("timeout".to_string()),
))
let result = match request.request_type {
RequestType::Execute => Callback::Execute(CallbackData::Error("timeout".to_string())),
RequestType::Query => Callback::Query(CallbackData::Error("timeout".to_string())),
};
Some(callback_msg(request, result))
}

#[cw_serde]
struct PendingCallback {
initiator: Addr,
initiator_msg: Binary,
receiver: Addr,
request_type: RequestType,
}

#[cw_serde]
pub enum RequestType {
Execute,
Query,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Created this type to we can figure out what type of callback we need to do.

Copy link
Contributor

Choose a reason for hiding this comment

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

maybe a little docstring?

/// Disambiguates between a callback for remote message execution and
/// queries.

}

const CALLBACKS: Map<u64, PendingCallback> = Map::new("polytone-callbacks");
Expand Down
2 changes: 1 addition & 1 deletion tests/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ require (
golang.org/x/text v0.7.0 // indirect
google.golang.org/genproto v0.0.0-20230125152338-dcaf20b6aeaa // indirect
google.golang.org/grpc v1.53.0 // indirect
google.golang.org/protobuf v1.28.2-0.20220831092852-f930b1dc76e8 // indirect
google.golang.org/protobuf v1.30.0 // indirect
gopkg.in/ini.v1 v1.67.0 // indirect
gopkg.in/yaml.v2 v2.4.0 // indirect
gopkg.in/yaml.v3 v3.0.1 // indirect
Expand Down
2 changes: 2 additions & 0 deletions tests/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1378,6 +1378,8 @@ google.golang.org/protobuf v1.26.0-rc.1/go.mod h1:jlhhOSvTdKEhbULTjvd4ARK9grFBp0
google.golang.org/protobuf v1.26.0/go.mod h1:9q0QmTI4eRPtz6boOQmLYwt+qCgq0jsYwAQnmE0givc=
google.golang.org/protobuf v1.28.2-0.20220831092852-f930b1dc76e8 h1:KR8+MyP7/qOlV+8Af01LtjL04bu7on42eVsxT4EyBQk=
google.golang.org/protobuf v1.28.2-0.20220831092852-f930b1dc76e8/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
google.golang.org/protobuf v1.30.0 h1:kPPoIgf3TsEvrm0PFe15JQ+570QVxYzEvvHqChK+cng=
google.golang.org/protobuf v1.30.0/go.mod h1:HV8QOd/L58Z+nl8r43ehVNZIU/HEI6OcFqwMG9pJV4I=
gopkg.in/alecthomas/kingpin.v2 v2.2.6/go.mod h1:FMv+mEhP44yOT+4EoQTLFTRgOQ1FBLkstjWtayDeSgw=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
Expand Down
30 changes: 29 additions & 1 deletion tests/simtests/contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (

"github.com/CosmWasm/wasmd/x/wasm/ibctesting"
sdk "github.com/cosmos/cosmos-sdk/types"
_ "github.com/cosmos/gogoproto/gogoproto"
)

type NoteInstantiate struct {
Expand Down Expand Up @@ -60,10 +61,37 @@ type CallbackMessage struct {
}

type Callback struct {
Success []string `json:"success,omitempty"`
Execute CallbackDataExecute `json:"execute,omitempty"`
Query CallbackDataQuery `json:"query,omitempty"`
}

type CallbackDataQuery struct {
Success [][]byte `json:"success,omitempty"`
Error string `json:"error,omitempty"`
}

type CallbackDataExecute struct {
Success []SubMsgResponse `json:"success,omitempty"`
Error string `json:"error,omitempty"`
}

type SubMsgResponse struct {
Events []Event `json:"events"`
Data []byte `json:"data,omitempty"`
}

type Events []Event
type Event struct {
Type string `json:"type"`
Attributes EventAttributes `json:"attributes"`
}

type EventAttributes []EventAttribute
type EventAttribute struct {
Key string `json:"key"`
Value string `json:"value"`
}

type Empty struct{}

type TesterQuery struct {
Expand Down
11 changes: 7 additions & 4 deletions tests/simtests/functionality_suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (c *Chain) MintBondedDenom(t *testing.T, to sdk.AccAddress) {
require.NoError(t, err)
}

func (s *Suite) RoundtripExecute(t *testing.T, path *ibctesting.Path, account *Account, msgs []any) (Callback, error) {
func (s *Suite) RoundtripExecute(t *testing.T, path *ibctesting.Path, account *Account, msgs []any) (CallbackDataExecute, error) {
Art3miX marked this conversation as resolved.
Show resolved Hide resolved
msg := NoteExecuteMsg{
Msgs: msgs,
TimeoutSeconds: 100,
Expand All @@ -101,12 +101,13 @@ func (s *Suite) RoundtripExecute(t *testing.T, path *ibctesting.Path, account *A
Msg: "aGVsbG8K",
},
}
return s.RoundtripMessage(t, path, account, NoteExecute{
callback, err := s.RoundtripMessage(t, path, account, NoteExecute{
Execute: &msg,
})
return callback.Execute, err
}

func (s *Suite) RoundtripQuery(t *testing.T, path *ibctesting.Path, account *Account, msgs []any) (Callback, error) {
func (s *Suite) RoundtripQuery(t *testing.T, path *ibctesting.Path, account *Account, msgs []any) (CallbackDataQuery, error) {
msg := NoteQuery{
Msgs: msgs,
TimeoutSeconds: 100,
Expand All @@ -115,9 +116,10 @@ func (s *Suite) RoundtripQuery(t *testing.T, path *ibctesting.Path, account *Acc
Msg: "aGVsbG8K",
},
}
return s.RoundtripMessage(t, path, account, NoteExecute{
callback, err := s.RoundtripMessage(t, path, account, NoteExecute{
Query: &msg,
})
return callback.Query, err
}

func (s *Suite) RoundtripMessage(t *testing.T, path *ibctesting.Path, account *Account, msg NoteExecute) (Callback, error) {
Expand All @@ -132,5 +134,6 @@ func (s *Suite) RoundtripMessage(t *testing.T, path *ibctesting.Path, account *A
callback := callbacks[len(callbacks)-1]
require.Equal(t, account.Address.String(), callback.Initiator)
require.Equal(t, "aGVsbG8K", callback.InitiatorMsg)

return callback.Result, nil
}
Loading