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

feat(08-wasm): implement 'RemoveCodeHash' RPC endpoint #5006

Merged
merged 26 commits into from
Nov 8, 2023
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
8c94bc5
feat: added proto types for RemoveCodeHash
srdtrk Nov 1, 2023
291ea46
imp: ran 'make proto-all'
srdtrk Nov 1, 2023
c2dd4de
feat: added implementation
srdtrk Nov 1, 2023
f25081c
test: implemented keeper_test with mockvm
srdtrk Oct 31, 2023
cfa906e
imp: added code hash validation
srdtrk Nov 1, 2023
0b1904c
test: added validation tests
srdtrk Oct 25, 2023
b013d3c
imp: added constructor and validate basic for remove code hash
srdtrk Nov 1, 2023
be11644
imp: tests
srdtrk Nov 2, 2023
e193251
test: added tests for msg_server
srdtrk Nov 2, 2023
d28ee70
merge: branch 'feat/wasm-clients' into serdar/issue#4951-remove-codehash
srdtrk Nov 2, 2023
d58e21e
deps: ran 'go mod tidy'
srdtrk Nov 2, 2023
6fe0d13
imp: added found to proto msg response
srdtrk Nov 2, 2023
a3df6e7
imp: ran 'make proto-all'
srdtrk Nov 2, 2023
e56c25b
imp: removed unused constant
srdtrk Nov 2, 2023
926ebab
imp: implemented the Found field
srdtrk Nov 2, 2023
5f3181b
imp: added more test cases
srdtrk Nov 2, 2023
7c9ee7c
merge: branch 'feat/wasm-clients' into serdar/issue#4951-remove-codehash
srdtrk Nov 7, 2023
acb1ea2
imp: added compiler assertions for MsgRemoveCodeHash
srdtrk Nov 7, 2023
4fd63aa
Merge branch 'feat/wasm-clients' into serdar/issue#4951-remove-codehash
srdtrk Nov 8, 2023
948106a
imp: review items
srdtrk Nov 8, 2023
9ae4353
imp: added msgs to codec
srdtrk Nov 8, 2023
dad6c1d
imp: removed found response from proto
srdtrk Nov 8, 2023
876df2d
imp: ran 'make proto-all'
srdtrk Nov 8, 2023
343dc62
fix: updated tests and logic to not use found
srdtrk Nov 8, 2023
234547c
imp: added elements match
srdtrk Nov 8, 2023
4eea37e
test: added codec tests
srdtrk Nov 8, 2023
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
17 changes: 17 additions & 0 deletions modules/light-clients/08-wasm/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm"
"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types"
ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
)
Expand All @@ -32,6 +33,22 @@ func (k Keeper) StoreCode(goCtx context.Context, msg *types.MsgStoreCode) (*type
}, nil
}

// RemoveCodeHash defines a rpc handler method for MsgRemoveCodeHash
func (k Keeper) RemoveCodeHash(goCtx context.Context, msg *types.MsgRemoveCodeHash) (*types.MsgRemoveCodeHashResponse, error) {
if k.GetAuthority() != msg.Signer {
return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "expected %s, got %s", k.GetAuthority(), msg.Signer)
}

found := types.HasCodeHash(goCtx, msg.CodeHash)
srdtrk marked this conversation as resolved.
Show resolved Hide resolved

err := ibcwasm.CodeHashes.Remove(goCtx, msg.CodeHash)
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
damiannolan marked this conversation as resolved.
Show resolved Hide resolved
if err != nil {
return nil, errorsmod.Wrap(err, "failed to remove code hash")
DimitrisJim marked this conversation as resolved.
Show resolved Hide resolved
}

return &types.MsgRemoveCodeHashResponse{Found: found}, nil
}

// MigrateContract defines a rpc handler method for MsgMigrateContract
func (k Keeper) MigrateContract(goCtx context.Context, msg *types.MsgMigrateContract) (*types.MsgMigrateContractResponse, error) {
if k.GetAuthority() != msg.Signer {
Expand Down
100 changes: 100 additions & 0 deletions modules/light-clients/08-wasm/keeper/msg_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import (
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"

"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/internal/ibcwasm"
wasmtesting "github.com/cosmos/ibc-go/modules/light-clients/08-wasm/testing"
"github.com/cosmos/ibc-go/modules/light-clients/08-wasm/types"
clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
Expand Down Expand Up @@ -278,3 +279,102 @@ func (suite *KeeperTestSuite) TestMsgMigrateContract() {
})
}
}

func (suite *KeeperTestSuite) TestMsgRemoveCodeHash() {
codeHash := sha256.Sum256(wasmtesting.Code)

govAcc := authtypes.NewModuleAddress(govtypes.ModuleName).String()

var (
msg *types.MsgRemoveCodeHash
expCodeHashes []types.CodeHash
expFound bool
)

testCases := []struct {
name string
malleate func()
expError error
}{
{
"success",
func() {
msg = types.NewMsgRemoveCodeHash(govAcc, codeHash[:])

expCodeHashes = []types.CodeHash{}
expFound = true
},
nil,
},
{
"success: many code hashes",
func() {
msg = types.NewMsgRemoveCodeHash(govAcc, codeHash[:])

expFound = true
expCodeHashes = []types.CodeHash{}

for i := 0; i < 20; i++ {
codeHash := sha256.Sum256([]byte{byte(i)})
err := ibcwasm.CodeHashes.Set(suite.chainA.GetContext(), codeHash[:])
suite.Require().NoError(err)

expCodeHashes = append(expCodeHashes, codeHash[:])
}
},
nil,
},
{
"success: code hash is missing",
func() {
msg = types.NewMsgRemoveCodeHash(govAcc, []byte{1})

expCodeHashes = []types.CodeHash{codeHash[:]}
expFound = false
},
nil,
},
{
"failure: unauthorized signer",
func() {
msg = types.NewMsgRemoveCodeHash(suite.chainA.SenderAccount.GetAddress().String(), codeHash[:])
},
ibcerrors.ErrUnauthorized,
},
}

for _, tc := range testCases {
suite.Run(tc.name, func() {
suite.SetupWasmWithMockVM()

endpoint := wasmtesting.NewWasmEndpoint(suite.chainA)
err := endpoint.CreateClient()
suite.Require().NoError(err)

tc.malleate()

ctx := suite.chainA.GetContext()
res, err := GetSimApp(suite.chainA).WasmClientKeeper.RemoveCodeHash(ctx, msg)
events := ctx.EventManager().Events().ToABCIEvents()

if tc.expError == nil {
suite.Require().NoError(err)
suite.Require().NotNil(res)
suite.Require().Equal(expFound, res.Found)

codeHashes, err := types.GetAllCodeHashes(suite.chainA.GetContext())
suite.Require().NoError(err)

// Check equality of code hashes up to order
suite.Require().Subset(expCodeHashes, codeHashes)
suite.Require().Subset(codeHashes, expCodeHashes)
srdtrk marked this conversation as resolved.
Show resolved Hide resolved

// Verify events
suite.Require().Len(events, 0)
Comment on lines +364 to +365
Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't add this necessarily but if we do we can atleast move getting the events in the successful case.

Copy link
Member Author

Choose a reason for hiding this comment

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

You think we should emit events?

Copy link
Member

Choose a reason for hiding this comment

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

is there actually 0 events emitted? hmm, would've thought sdk baseapp would emit some std ones

Copy link
Member Author

Choose a reason for hiding this comment

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

The context is not going through the baseapp. I'm directly calling the msg_server handler with the context

Copy link
Member

Choose a reason for hiding this comment

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

makes sense!

Copy link
Contributor

Choose a reason for hiding this comment

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

not sure re events. I dont think they'd hurt tho.

} else {
suite.Require().ErrorIs(err, tc.expError)
suite.Require().Nil(res)
}
})
}
}
26 changes: 26 additions & 0 deletions modules/light-clients/08-wasm/types/msgs.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ import (
var (
_ sdk.Msg = (*MsgStoreCode)(nil)
_ sdk.Msg = (*MsgMigrateContract)(nil)
_ sdk.Msg = (*MsgRemoveCodeHash)(nil)
_ sdk.HasValidateBasic = (*MsgStoreCode)(nil)
_ sdk.HasValidateBasic = (*MsgMigrateContract)(nil)
_ sdk.HasValidateBasic = (*MsgRemoveCodeHash)(nil)
)

// MsgStoreCode creates a new MsgStoreCode instance
Expand All @@ -37,6 +39,30 @@ func (m MsgStoreCode) ValidateBasic() error {
return nil
}

// NewMsgRemoveCodeHash creates a new MsgRemoveCodeHash instance
//
//nolint:interfacer
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
func NewMsgRemoveCodeHash(signer string, codeHash []byte) *MsgRemoveCodeHash {
return &MsgRemoveCodeHash{
Signer: signer,
CodeHash: codeHash,
}
}

// ValidateBasic implements sdk.Msg
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
func (m MsgRemoveCodeHash) ValidateBasic() error {
_, err := sdk.AccAddressFromBech32(m.Signer)
if err != nil {
return errorsmod.Wrapf(ibcerrors.ErrInvalidAddress, "string could not be parsed as address: %v", err)
}

if err := ValidateWasmCodeHash(m.CodeHash); err != nil {
return err
}

return nil
}

// MsgMigrateContract creates a new MsgMigrateContract instance
func NewMsgMigrateContract(signer, clientID string, codeHash, migrateMsg []byte) *MsgMigrateContract {
return &MsgMigrateContract{
Expand Down
49 changes: 47 additions & 2 deletions modules/light-clients/08-wasm/types/msgs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ func TestMsgStoreCodeValidateBasic(t *testing.T) {
err := tc.msg.ValidateBasic()
expPass := tc.expErr == nil
if expPass {
require.NoError(t, err)
require.NoError(t, err, tc.name)
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
} else {
require.ErrorIs(t, err, tc.expErr)
}
Expand Down Expand Up @@ -153,7 +153,52 @@ func TestMsgMigrateContractValidateBasic(t *testing.T) {
if expPass {
require.NoError(t, err)
} else {
require.ErrorIs(t, err, tc.expErr)
require.ErrorIs(t, err, tc.expErr, tc.name)
}
}
}

func TestMsgRemoveCodeHashValidateBasic(t *testing.T) {
signer := sdk.AccAddress(ibctesting.TestAccAddress).String()

codeHash := sha256.Sum256(wasmtesting.Code)

testCases := []struct {
name string
msg *types.MsgRemoveCodeHash
expErr error
}{
{
"success: valid signer address, valid length code hash",
types.NewMsgRemoveCodeHash(signer, codeHash[:]),
nil,
},
{
"failure: code hash is empty",
types.NewMsgRemoveCodeHash(signer, []byte("")),
types.ErrInvalidCodeHash,
},
{
"failure: code hash is nil",
types.NewMsgRemoveCodeHash(signer, nil),
types.ErrInvalidCodeHash,
},
{
"failure: signer is invalid",
types.NewMsgRemoveCodeHash(ibctesting.InvalidID, codeHash[:]),
ibcerrors.ErrInvalidAddress,
},
}

for _, tc := range testCases {
tc := tc

err := tc.msg.ValidateBasic()
expPass := tc.expErr == nil
srdtrk marked this conversation as resolved.
Show resolved Hide resolved
if expPass {
require.NoError(t, err, tc.name)
} else {
require.ErrorIs(t, err, tc.expErr, tc.name)
}
}
}
Loading
Loading