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

Packet Ack not correct #47

Closed
GNaD13 opened this issue Jan 23, 2023 · 2 comments · Fixed by #51
Closed

Packet Ack not correct #47

GNaD13 opened this issue Jan 23, 2023 · 2 comments · Fixed by #51
Assignees

Comments

@GNaD13
Copy link
Contributor

GNaD13 commented Jan 23, 2023

ISSUE
Currently, packet ack reflect from contract not correct. It only respone value(which is the raw data spot price from osmosis) here

let res = match deps.querier.raw_query(&raw) {
        SystemResult::Err(system_err) => Err(StdError::generic_err(format!(
            "Querier contract error: {}",
            system_err
        ))),
        SystemResult::Ok(ContractResult::Err(contract_err)) => Err(StdError::generic_err(format!(
            "Querier contract error: {}",
            contract_err
        ))),
        SystemResult::Ok(ContractResult::Ok(value)) => Ok(IbcReceiveResponse::<Empty>::new()
            .set_ack(value)
            .add_attribute("action", "receive_ibc_query")),
   };

=> To read this data, we just need to have json unmarshal with types.SpotPrice (Current running in vuong/testingquery branch).

var ack channeltypes.Acknowledgement
// TODO: Handler ack logic here
// TODO : update spot price when receive ack from osmosis chain
bz := acknowledgement
spotPrice, err := am.keeper.UnmarshalPacketBytesToPrice(bz)
if err != nil {
return err
}

We still can read this type of data but the thing is it may meet problem if the ibc reflect packet is error.
The value should be embedded in SDK ACK packet struct (channeltypes.Acknowledgement) so feeabs chain module can handle the ibc ack packet.

SOLUTION

  • Contract: Embedded spot price value in ibc ack packet
let acknowledgement = to_binary(&AcknowledgementMsg::Ok(value))?;
    Ok(IbcReceiveResponse::new()
        .set_ack(acknowledgement)
        .add_attribute("key_attr", "attr_value"))
  • Add check ack
var ack channeltypes.Acknowledgement

if err := types.ModuleCdc.UnmarshalJSON(acknowledgement, &ack); err != nil {
	return sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "cannot unmarshal packet acknowledgement: %v", err)
}

switch resp := ack.Response.(type) {
case *channeltypes.Acknowledgement_Result:
    // Ack success logic
case *channeltypes.Acknowledgement_Error:
    // Ack error logic
}
@GNaD13
Copy link
Contributor Author

GNaD13 commented Jan 23, 2023

@expertdicer pls check this issue, can you fix with contract side pls? I will handle with the chain side.
@vuong177 some minor issue, but i think we can still use current version for demo. The result is unaffected

@expertdicer
Copy link
Contributor

Done this with notional-labs/feeabstraction-contract#6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants