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

Missing Amino Codec Registration #736

Merged
merged 8 commits into from
Oct 19, 2022
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/foundation) [\#704](https://github.com/line/lbm-sdk/pull/704) update x/foundation params
* (x/wasm) [\#695](https://github.com/line/lbm-sdk/pull/695) fix to prevent external filesystem dependency of simulation
* (x/foundation) [\#729](https://github.com/line/lbm-sdk/pull/729) add UpdateParams to x/foundation
* (amino) [\#736](https://github.com/line/lbm-sdk/pull/736) apply the missing amino codec registratoin of cosmos-sdk

### Bug Fixes
* (x/wasm) [\#453](https://github.com/line/lbm-sdk/pull/453) modify wasm grpc query api path
Expand Down
11 changes: 6 additions & 5 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
dbm "github.com/tendermint/tm-db"

"github.com/line/lbm-sdk/codec"
"github.com/line/lbm-sdk/codec/legacy"
"github.com/line/lbm-sdk/snapshots"
snapshottypes "github.com/line/lbm-sdk/snapshots/types"
"github.com/line/lbm-sdk/store/rootmulti"
Expand Down Expand Up @@ -91,10 +92,10 @@ func registerTestCodec(cdc *codec.LegacyAmino) {

// register test types
cdc.RegisterConcrete(&txTest{}, "cosmos-sdk/baseapp/txTest", nil)
cdc.RegisterConcrete(&msgCounter{}, "cosmos-sdk/baseapp/msgCounter", nil)
cdc.RegisterConcrete(&msgCounter2{}, "cosmos-sdk/baseapp/msgCounter2", nil)
cdc.RegisterConcrete(&msgKeyValue{}, "cosmos-sdk/baseapp/msgKeyValue", nil)
cdc.RegisterConcrete(&msgNoRoute{}, "cosmos-sdk/baseapp/msgNoRoute", nil)
legacy.RegisterAminoMsg(cdc, &msgCounter{}, "cosmos-sdk/baseapp/msgCounter")
legacy.RegisterAminoMsg(cdc, &msgCounter2{}, "cosmos-sdk/baseapp/msgCounter2")
legacy.RegisterAminoMsg(cdc, &msgKeyValue{}, "cosmos-sdk/baseapp/msgKeyValue")
legacy.RegisterAminoMsg(cdc, &msgNoRoute{}, "cosmos-sdk/baseapp/msgNoRoute")
}

// aminoTxEncoder creates a amino TxEncoder for testing purposes.
Expand Down Expand Up @@ -1257,7 +1258,7 @@ func TestRunInvalidTransaction(t *testing.T) {
// new codec so we can encode the tx, but we shouldn't be able to decode
newCdc := codec.NewLegacyAmino()
registerTestCodec(newCdc)
newCdc.RegisterConcrete(&msgNoDecode{}, "cosmos-sdk/baseapp/msgNoDecode", nil)
legacy.RegisterAminoMsg(newCdc, &msgNoDecode{}, "cosmos-sdk/baseapp/msgNoDecode")

txBytes, err := newCdc.Marshal(tx)
require.NoError(t, err)
Expand Down
18 changes: 18 additions & 0 deletions codec/legacy/amino_msg.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package legacy

import (
"fmt"

"github.com/line/lbm-sdk/codec"
sdk "github.com/line/lbm-sdk/types"
)

// RegisterAminoMsg first checks that the msgName is <40 chars
// (else this would break ledger nano signing: https://github.com/cosmos/cosmos-sdk/issues/10870),
// then registers the concrete msg type with amino.
func RegisterAminoMsg(cdc *codec.LegacyAmino, msg sdk.Msg, msgName string) {
if len(msgName) > 39 {
panic(fmt.Errorf("msg name %s is too long to be registered with amino", msgName))
}
cdc.RegisterConcrete(msg, msgName, nil)
}
39 changes: 39 additions & 0 deletions codec/legacy/amino_msg_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package legacy_test

import (
"strings"
"testing"

"github.com/stretchr/testify/require"

"github.com/line/lbm-sdk/codec"
"github.com/line/lbm-sdk/codec/legacy"
"github.com/line/lbm-sdk/testutil/testdata"
)

func TestRegisterAminoMsg(t *testing.T) {
cdc := codec.NewLegacyAmino()

testCases := map[string]struct {
msgName string
expPanic bool
}{
"all good": {
msgName: "cosmos-sdk/Test",
},
"msgName too long": {
msgName: strings.Repeat("a", 40),
expPanic: true,
},
}
for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
fn := func() { legacy.RegisterAminoMsg(cdc, &testdata.TestMsg{}, tc.msgName) }
if tc.expPanic {
require.Panics(t, fn)
} else {
require.NotPanics(t, fn)
}
})
}
}
5 changes: 3 additions & 2 deletions codec/legacy/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,19 @@ import (
"github.com/line/lbm-sdk/codec"
cryptocodec "github.com/line/lbm-sdk/crypto/codec"
cryptotypes "github.com/line/lbm-sdk/crypto/types"
sdk "github.com/line/lbm-sdk/types"
)

// Cdc defines a global generic sealed Amino codec to be used throughout sdk. It
// has all Tendermint crypto and evidence types registered.
//
// TODO: Deprecated - remove this global.
var Cdc *codec.LegacyAmino
var Cdc = codec.NewLegacyAmino()

func init() {
Cdc = codec.NewLegacyAmino()
cryptocodec.RegisterCrypto(Cdc)
codec.RegisterEvidences(Cdc)
sdk.RegisterLegacyAminoCodec(Cdc)
}

// PrivKeyFromBytes unmarshals private key bytes and returns a PrivKey
Expand Down
2 changes: 2 additions & 0 deletions codec/legacy/doc.go
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
// Package legacy contains a global amino Cdc which is deprecated but
// still used in several places within the SDK. This package is intended
// to be removed at some point in the future when the global Cdc is removed.
// It also contains a util function RegisterAminoMsg that checks a msg name length
// before registering the concrete msg type with amino.
package legacy
Loading