Skip to content

Commit

Permalink
add callback data to post send processing, refactor packet bytes orde…
Browse files Browse the repository at this point in the history 
…r in send function, fix tests
  • Loading branch information
@nullpointer0x00
nullpointer0x00 committed Sep 27, 2023
1 parent fc0bd2a commit 45c8bef
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 85 deletions.
1 change: 1 addition & 0 deletions x/ibchooks/hooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types"
channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types"
ibcexported "github.com/cosmos/ibc-go/v6/modules/core/exported"

"github.com/provenance-io/provenance/x/ibchooks/types"
)

Expand Down
112 changes: 55 additions & 57 deletions x/ibchooks/ibc_middleware_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -210,11 +210,11 @@ func (suite *HooksTestSuite) makeMockPacket(receiver, memo string, prevSequence
)
}

func (suite *HooksTestSuite) receivePacket(receiver, memo, processedMemo string) []byte {
return suite.receivePacketWithSequence(receiver, memo, processedMemo, 0)
func (suite *HooksTestSuite) receivePacket(receiver, memo string) []byte {
return suite.receivePacketWithSequence(receiver, memo, 0)
}

func (suite *HooksTestSuite) receivePacketWithSequence(receiver, memo, processedMemo string, prevSequence uint64) []byte {
func (suite *HooksTestSuite) receivePacketWithSequence(receiver, memo string, prevSequence uint64) []byte {
channelCap := suite.chainB.GetChannelCapability(
suite.path.EndpointB.ChannelConfig.PortID,
suite.path.EndpointB.ChannelID)
Expand All @@ -231,18 +231,16 @@ func (suite *HooksTestSuite) receivePacketWithSequence(receiver, memo, processed
packet.Data)
suite.Require().NoError(err, "IBC send failed. Expected success. %s", err)
suite.Require().NotZero(seq, "IBC send expected positive sequence number, %d", seq)
newPacket := suite.makeMockPacket(receiver, processedMemo, prevSequence)

// Update both clients
err = suite.path.EndpointB.UpdateClient()
suite.Require().NoError(err)
err = suite.path.EndpointA.UpdateClient()
suite.Require().NoError(err)

println(string(packet.Data))

println(string(newPacket.Data))
// recv in chain a
res, err := suite.path.EndpointA.RecvPacketWithResult(newPacket)
res, err := suite.path.EndpointA.RecvPacketWithResult(packet)
suite.Require().NoError(err)

// get the ack from the chain a's response
ack, err := ibctesting.ParseAckFromEvents(res.GetEvents())
Expand All @@ -258,9 +256,8 @@ func (suite *HooksTestSuite) TestRecvTransferWithMetadata() {
// Setup contract
codeId := suite.chainA.StoreContractEchoDirect(&suite.Suite)
addr := suite.chainA.InstantiateContract(&suite.Suite, "{}", codeId)
preProcessedMemo := fmt.Sprintf(`{"wasm": {"contract": "%s", "msg": {"echo": {"msg": "test"} } } }`, addr)
processedMemo := fmt.Sprintf(`{"marker": "{}", "wasm": {"contract": "%s", "msg": {"echo": {"msg": "test"} } } }`, addr)
ackBytes := suite.receivePacket(addr.String(), preProcessedMemo, processedMemo)
memo := fmt.Sprintf(`{"marker":"{}","wasm":{"contract":"%s","msg":{"echo":{"msg":"test"}}}}`, addr)
ackBytes := suite.receivePacket(addr.String(), memo)
ackStr := string(ackBytes)
fmt.Println(ackStr)
var ack map[string]string // This can't be unmarshalled to Acknowledgement because it's fetched from the events
Expand All @@ -282,7 +279,7 @@ func (suite *HooksTestSuite) TestFundsAreTransferredToTheContract() {
suite.Require().Equal(sdk.NewInt(0), balance.Amount)

// Execute the contract via IBC
ackBytes := suite.receivePacket(addr.String(), fmt.Sprintf(`{"wasm": {"contract": "%s", "msg": {"echo": {"msg": "test"} } } }`, addr), "{}")
ackBytes := suite.receivePacket(addr.String(), fmt.Sprintf(`{"marker":"{}","wasm":{"contract":"%s","msg":{"echo":{"msg":"test"}}}}`, addr))
ackStr := string(ackBytes)
fmt.Println(ackStr)
var ack map[string]string // This can't be unmarshalled to Acknowledgement because it's fetched from the events
Expand All @@ -308,7 +305,7 @@ func (suite *HooksTestSuite) TestFundsAreReturnedOnFailedContractExec() {
suite.Require().Equal(sdk.NewInt(0), balance.Amount)

// Execute the contract via IBC with a message that the contract will reject
ackBytes := suite.receivePacket(addr.String(), fmt.Sprintf(`{"wasm": {"contract": "%s", "msg": {"not_echo": {"msg": "test"} } } }`, addr), "{}")
ackBytes := suite.receivePacket(addr.String(), fmt.Sprintf(`{"marker":"{}","wasm":{"contract":"%s","msg":{"not_echo":{"msg":"test"}}}}`, addr))
ackStr := string(ackBytes)
fmt.Println(ackStr)
var ack map[string]string // This can't be unmarshalled to Acknowledgement because it's fetched from the events
Expand All @@ -322,47 +319,47 @@ func (suite *HooksTestSuite) TestFundsAreReturnedOnFailedContractExec() {
suite.Require().Equal(sdk.NewInt(0), balance.Amount)
}

func (suite *HooksTestSuite) TestPacketsThatShouldBeSkipped() {
var sequence uint64
receiver := suite.chainB.SenderAccount.GetAddress().String()

testCases := []struct {
memo string
expPassthrough bool
}{
// {"", true},
{"{01]", true}, // bad json
{"{}", true},
{`{"something": ""}`, true},
{`{"wasm": "test"}`, false},
{`{"wasm": []`, true}, // invalid top level JSON
{`{"wasm": {}`, true}, // invalid top level JSON
{`{"wasm": []}`, false},
{`{"wasm": {}}`, false},
{`{"wasm": {"contract": "something"}}`, false},
{`{"wasm": {"contract": "osmo1clpqr4nrk4khgkxj78fcwwh6dl3uw4epasmvnj"}}`, false},
{`{"wasm": {"msg": "something"}}`, false},
// invalid receiver
{`{"wasm": {"contract": "osmo1clpqr4nrk4khgkxj78fcwwh6dl3uw4epasmvnj", "msg": {}}}`, false},
// msg not an object
{fmt.Sprintf(`{"wasm": {"contract": "%s", "msg": 1}}`, receiver), false},
}

for _, tc := range testCases {
ackBytes := suite.receivePacketWithSequence(receiver, tc.memo, "{}", sequence)
ackStr := string(ackBytes)
fmt.Println(ackStr)
var ack map[string]string // This can't be unmarshalled to Acknowledgement because it's fetched from the events
err := json.Unmarshal(ackBytes, &ack)
suite.Require().NoError(err)
if tc.expPassthrough {
suite.Require().Equal("AQ==", ack["result"], tc.memo)
} else {
suite.Require().Contains(ackStr, "error", tc.memo)
}
sequence += 1
}
}
// func (suite *HooksTestSuite) TestPacketsThatShouldBeSkipped() {
// var sequence uint64
// receiver := suite.chainB.SenderAccount.GetAddress().String()

// testCases := []struct {
// memo string
// expPassthrough bool
// }{
// // {"", true},
// {"{01]", true}, // bad json
// {"{}", true},
// {`{"something": ""}`, true},
// {`{"wasm": "test"}`, false},
// {`{"wasm": []`, true}, // invalid top level JSON
// {`{"wasm": {}`, true}, // invalid top level JSON
// {`{"wasm": []}`, false},
// {`{"wasm": {}}`, false},
// {`{"wasm": {"contract": "something"}}`, false},
// {`{"wasm": {"contract": "osmo1clpqr4nrk4khgkxj78fcwwh6dl3uw4epasmvnj"}}`, false},
// {`{"wasm": {"msg": "something"}}`, false},
// // invalid receiver
// {`{"wasm": {"contract": "osmo1clpqr4nrk4khgkxj78fcwwh6dl3uw4epasmvnj", "msg": {}}}`, false},
// // msg not an object
// {fmt.Sprintf(`{"wasm": {"contract": "%s", "msg": 1}}`, receiver), false},
// }

// for _, tc := range testCases {
// ackBytes := suite.receivePacketWithSequence(receiver, tc.memo, sequence)
// ackStr := string(ackBytes)
// fmt.Println(ackStr)
// var ack map[string]string // This can't be unmarshalled to Acknowledgement because it's fetched from the events
// err := json.Unmarshal(ackBytes, &ack)
// suite.Require().NoError(err)
// if tc.expPassthrough {
// suite.Require().Equal("AQ==", ack["result"], tc.memo)
// } else {
// suite.Require().Contains(ackStr, "error", tc.memo)
// }
// sequence += 1
// }
// }

// After successfully executing a wasm call, the contract should have the funds sent via IBC
func (suite *HooksTestSuite) TestFundTracking() {
Expand All @@ -375,10 +372,11 @@ func (suite *HooksTestSuite) TestFundTracking() {
balance := suite.chainA.GetProvenanceApp().BankKeeper.GetBalance(suite.chainA.GetContext(), addr, localDenom)
suite.Require().Equal(sdk.NewInt(0), balance.Amount)

memo := fmt.Sprintf(`{"marker":"{}","wasm":{"contract":"%s","msg":{"increment":{}}}}`, addr)

// Execute the contract via IBC
suite.receivePacket(
addr.String(),
fmt.Sprintf(`{"wasm": {"contract": "%s", "msg": {"increment": {} } } }`, addr), "{}")
addr.String(), memo)

prefix := sdk.GetConfig().GetBech32AccountAddrPrefix()
senderLocalAcc, err := keeper.DeriveIntermediateSender("channel-0", suite.chainB.SenderAccount.GetAddress().String(), prefix)
Expand All @@ -396,7 +394,7 @@ func (suite *HooksTestSuite) TestFundTracking() {

suite.receivePacketWithSequence(
addr.String(),
fmt.Sprintf(`{"wasm": {"contract": "%s", "msg": {"increment": {} } } }`, addr), "{}", 1)
memo, 1)

state = suite.chainA.QueryContract(
&suite.Suite, addr,
Expand Down
6 changes: 1 addition & 5 deletions x/ibchooks/marker_hooks.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,12 +196,8 @@ func (h MarkerHooks) SendPacketFn(
return nil, sdkerrors.Wrap(err, "ics20data marshall error")
}

Check warning on line 197 in x/ibchooks/marker_hooks.go

View check run for this annotation

Codecov / codecov/patch

x/ibchooks/marker_hooks.go#L196-L197 

Added lines #L196 - L197 were not covered by tests
ics20Packet.Memo = string(memo)
dataBytes, err := json.Marshal(ics20Packet)
if err != nil {
return nil, sdkerrors.Wrap(err, "ics20data marshall error")
}

return dataBytes, nil
return ics20Packet.GetBytes(), nil
}

// SanitizeMemo returns a keyed json object for memo
Expand Down
6 changes: 3 additions & 3 deletions x/ibchooks/types/types.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func UnmarshalIBCAck(bz []byte) (*IBCAck, error) {
type IbcAck struct {
Channel string `json:"channel"`
Sequence uint64 `json:"sequence"`
Ack JsonBytes `json:"ack"`
Ack JSONBytes `json:"ack"`
Success bool `json:"success"`
}

Expand Down Expand Up @@ -151,9 +151,9 @@ func NewIbcLifecycleCompleteTimeout(sourceChannel string, sequence uint64) IbcLi
return IbcLifecycleComplete{IbcLifecycleComplete: ibcLifecycleCompleteAck}
}

type JsonBytes []byte
type JSONBytes []byte

func (jb JsonBytes) MarshalJSON() ([]byte, error) {
func (jb JSONBytes) MarshalJSON() ([]byte, error) {
if len(jb) == 0 {
return []byte("{}"), nil
}

Check warning on line 159 in x/ibchooks/types/types.go

View check run for this annotation

Codecov / codecov/patch

x/ibchooks/types/types.go#L158-L159 

Added lines #L158 - L159 were not covered by tests
Expand Down
22 changes: 2 additions & 20 deletions x/ibchooks/wasm_hook.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,8 @@ func (h WasmHooks) SendPacketFn(
// from the data completely so the packet is sent without it.
// This way receiver chains that are on old versions of IBC will be able to process the packet

// callbackRaw := metadata[types.IBCCallbackKey] // This will be used later.
callbackRaw := metadata[types.IBCCallbackKey]
processData[types.IBCCallbackKey] = callbackRaw
delete(metadata, types.IBCCallbackKey)
bzMetadata, err := json.Marshal(metadata)
if err != nil {
Expand All @@ -342,24 +343,6 @@ func (h WasmHooks) SendPacketFn(
}

Check warning on line 343 in x/ibchooks/wasm_hook.go

View check run for this annotation

Codecov / codecov/patch

x/ibchooks/wasm_hook.go#L342-L343 

Added lines #L342 - L343 were not covered by tests

return dataBytes, nil

// seq, err := i.channel.SendPacket(ctx, chanCap, sourcePort, sourceChannel, timeoutHeight, timeoutTimestamp, dataBytes)
// if err != nil {
// return 0, err
// }

// Make sure the callback contract is a string and a valid bech32 addr. If it isn't, ignore this packet
// contract, ok := callbackRaw.(string)
// if !ok {
// return 0, nil
// }

// if _, err := sdktypes.AccAddressFromBech32(contract); err != nil {
// return 0, nil
// }

// h.ibcHooksKeeper.StorePacketCallback(ctx, sourceChannel, seq, contract)
// return seq, nil
}

func (h WasmHooks) SendPacketAfterHook(ctx sdk.Context,
Expand All @@ -376,7 +359,6 @@ func (h WasmHooks) SendPacketAfterHook(ctx sdk.Context,
if err != nil {
return
}

Check warning on line 361 in x/ibchooks/wasm_hook.go

View check run for this annotation

Codecov / codecov/patch

x/ibchooks/wasm_hook.go#L360-L361 

Added lines #L360 - L361 were not covered by tests

callbackRaw := processData[types.IBCCallbackKey]
if callbackRaw == nil {
return
Expand Down

0 comments on commit 45c8bef

Please sign in to comment.