refactor: update IncentivzedAck + add channeltype helpers for Ack #749
Conversation
… helpers for unpack/pack Any
seantking
requested review from
charleenfei,
AdityaSripal,
crodriguezvega,
damiannolan and
colin-axner
|
|
||
| // UnpackAcknowledgement unpacks an Any into an Acknowledgement. It returns an error if the | ||
| // acknowledgement can't be unpacked into a Acknowledgement. | ||
| func UnpackAcknowledgement(any *codectypes.Any) (exported.Acknowledgement, error) { |
There was a problem hiding this comment.
@colin-axner I will make a follow-up PR with these changes into main if we decide to go this route.
modules/apps/29-fee/keeper/relay.go
Outdated
| @@ -23,8 +23,15 @@ func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.C | |||
|
|
|||
| k.DeleteForwardRelayerAddress(ctx, packetId) | |||
|
|
|||
| ack := types.NewIncentivizedAcknowledgement(relayer, acknowledgement) | |||
| bz := ack.Acknowledgement() | |||
| ack := channeltypes.Acknowledgement{} | |||
There was a problem hiding this comment.
@AdityaSripal Is this what you meant?
There was a problem hiding this comment.
update the WriteAcknowledgement API to take in exported.Acknowledgement and then the code will look good
|
I have made this a draft PR for now as I'm unsure if this is the approach we want to take and I would like some early feedback. I have added @colin-axner @damiannolan feedback appreciated. |
| // IncentivizedAcknowledgement is the acknowledgement format to be used by applications wrapped in the fee middleware | ||
| // It contains the raw acknowledgement bytes, as well as the forward relayer address | ||
| message IncentivizedAcknowledgement { | ||
| bytes result = 1; | ||
| google.protobuf.Any app_acknowledgement = 1 [(gogoproto.moretags) = "yaml:\"forward_relayer_address\""]; |
| @@ -16,7 +19,12 @@ func NewIncentivizedAcknowledgement(relayer string, ack []byte) IncentivizedAckn | |||
| // considered successful if the forward relayer address is empty. Otherwise it is | |||
| // considered a failed acknowledgement. | |||
| func (ack IncentivizedAcknowledgement) Success() bool { | |||
There was a problem hiding this comment.
I think IncentivizedAcknowledgement needs to implement UnpackInterfaces. It might be best to write a test first to determine if this is true. UnpackInterfaces is used when JSON marshalling/unmarshalling an Any. That's why we have this test case
There was a problem hiding this comment.
I have implemented this but it seems to not be working 🤔 Getting some failed tests for the reason you mention here.
modules/apps/29-fee/keeper/relay.go
Outdated
| @@ -23,8 +23,15 @@ func (k Keeper) WriteAcknowledgement(ctx sdk.Context, chanCap *capabilitytypes.C | |||
|
|
|||
| k.DeleteForwardRelayerAddress(ctx, packetId) | |||
|
|
|||
| ack := types.NewIncentivizedAcknowledgement(relayer, acknowledgement) | |||
| bz := ack.Acknowledgement() | |||
| ack := channeltypes.Acknowledgement{} | |||
There was a problem hiding this comment.
update the WriteAcknowledgement API to take in exported.Acknowledgement and then the code will look good
|
Closing in favor of : #952 |
Description
closes: #XXXX
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes