From 00a5627103adc6cba707bc86d40328c4de945106 Mon Sep 17 00:00:00 2001 From: egonspace Date: Wed, 15 Sep 2021 09:37:49 +0900 Subject: [PATCH 01/16] test: enable race test --- .github/workflows/test.yml | 43 +++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 22 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c37ac4d5ae..c98b6f9ba2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -65,28 +65,27 @@ jobs: - name: Build run: GOOS=linux CGO_ENABLED=1 GOARCH=${{ matrix.goarch }} CC=${{ matrix.gcc }} LEDGER_ENABLED=false make build - # TODO: disable test-race. please enable this after fixing concurrent checkTx - # test-cosmovisor: - # runs-on: ubuntu-latest - # steps: - # - uses: actions/checkout@v2 - # - uses: actions/setup-go@v2.1.4 - # with: - # go-version: 1.15 - # - name: Display go version - # run: go version - # - uses: technote-space/get-diff-action@v5.0.1 - # id: git_diff - # with: - # PREFIX_FILTER: | - # cosmovisor - # PATTERNS: | - # **/**.go - # go.mod - # go.sum - # - name: Run cosmovisor tests - # run: cd cosmovisor; make - # if: env.GIT_DIFF + test-cosmovisor: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - uses: actions/setup-go@v2.1.4 + with: + go-version: 1.15 + - name: Display go version + run: go version + - uses: technote-space/get-diff-action@v5.0.1 + id: git_diff + with: + PREFIX_FILTER: | + cosmovisor + PATTERNS: | + **/**.go + go.mod + go.sum + - name: Run cosmovisor tests + run: cd cosmovisor; make + if: env.GIT_DIFF split-test-files: runs-on: ubuntu-latest From 798eb460ffd343ce7a2fd9809144c2dcf5ac0c2e Mon Sep 17 00:00:00 2001 From: egonspace Date: Wed, 15 Sep 2021 09:51:29 +0900 Subject: [PATCH 02/16] test: enable sim test --- .github/workflows/sims.yml | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/.github/workflows/sims.yml b/.github/workflows/sims.yml index 15ae3ffbc9..24c62dfcb5 100644 --- a/.github/workflows/sims.yml +++ b/.github/workflows/sims.yml @@ -4,12 +4,11 @@ name: Sims env: GOPRIVATE: "github.com/line/*" -# TODO ebony: fix sim test failure on: -# pull_request: -# push: -# branches: -# - main + pull_request: + push: + branches: + - main jobs: cleanup-runs: From 4331513c4199904198220c41914209b07868d457 Mon Sep 17 00:00:00 2001 From: egonspace Date: Thu, 16 Sep 2021 08:03:52 +0900 Subject: [PATCH 03/16] fix: implement MarshalJSONPB --- x/auth/vesting/types/vesting_account.go | 174 ++++++++++++++++++++++++ 1 file changed, 174 insertions(+) diff --git a/x/auth/vesting/types/vesting_account.go b/x/auth/vesting/types/vesting_account.go index b4b1f66853..47f10f4ea8 100644 --- a/x/auth/vesting/types/vesting_account.go +++ b/x/auth/vesting/types/vesting_account.go @@ -1,9 +1,12 @@ package types import ( + "encoding/json" "errors" + "strings" "time" + "github.com/gogo/protobuf/jsonpb" yaml "gopkg.in/yaml.v2" sdk "github.com/line/lbm-sdk/types" @@ -209,6 +212,54 @@ func (bva BaseVestingAccount) MarshalYAML() (interface{}, error) { return string(bz), err } +type vestingAccountJSON struct { + BaseAccount json.RawMessage `json:"base_account"` + OriginalVesting sdk.Coins `json:"original_vesting"` + DelegatedFree sdk.Coins `json:"delegated_free"` + DelegatedVesting sdk.Coins `json:"delegated_vesting"` + EndTime int64 `json:"end_time"` + + StartTime int64 `json:"start_time,omitempty"` + VestingPeriods Periods `json:"vesting_periods,omitempty"` +} + +func (bva BaseVestingAccount) MarshalJSONPB(m *jsonpb.Marshaler) ([]byte, error) { + bz, err := bva.BaseAccount.MarshalJSONPB(m) + if err != nil { + return nil, err + } + alias := vestingAccountJSON{ + BaseAccount: bz, + OriginalVesting: bva.OriginalVesting, + DelegatedFree: bva.DelegatedFree, + DelegatedVesting: bva.DelegatedVesting, + EndTime: bva.EndTime, + } + + return json.Marshal(alias) +} + +func (bva *BaseVestingAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz []byte) error { + var va vestingAccountJSON + + err := json.Unmarshal(bz, &va) + if err != nil { + return err + } + + ba := new(authtypes.BaseAccount) + if err := m.Unmarshal(strings.NewReader(string(va.BaseAccount)), ba); err != nil { + return err + } + bva.BaseAccount = ba + bva.OriginalVesting = va.OriginalVesting + bva.DelegatedFree = va.DelegatedFree + bva.OriginalVesting = va.DelegatedVesting + bva.EndTime = va.EndTime + + return nil +} + //----------------------------------------------------------------------------- // Continuous Vesting Account @@ -332,6 +383,47 @@ func (cva ContinuousVestingAccount) MarshalYAML() (interface{}, error) { return string(bz), err } +func (cva ContinuousVestingAccount) MarshalJSONPB(m *jsonpb.Marshaler) ([]byte, error) { + bz, err := cva.BaseVestingAccount.BaseAccount.MarshalJSONPB(m) + if err != nil { + return nil, err + } + alias := vestingAccountJSON{ + BaseAccount: bz, + OriginalVesting: cva.BaseVestingAccount.OriginalVesting, + DelegatedFree: cva.BaseVestingAccount.DelegatedFree, + DelegatedVesting: cva.BaseVestingAccount.DelegatedVesting, + EndTime: cva.BaseVestingAccount.EndTime, + StartTime: cva.StartTime, + } + + return json.Marshal(alias) +} + +func (cva *ContinuousVestingAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz []byte) error { + var va vestingAccountJSON + + err := json.Unmarshal(bz, &va) + if err != nil { + return err + } + + ba := new(authtypes.BaseAccount) + if err := m.Unmarshal(strings.NewReader(string(va.BaseAccount)), ba); err != nil { + return err + } + cva.BaseVestingAccount = &BaseVestingAccount{ + BaseAccount: ba, + OriginalVesting: va.OriginalVesting, + DelegatedFree: va.DelegatedFree, + DelegatedVesting: va.DelegatedVesting, + EndTime: va.EndTime, + } + cva.StartTime = va.StartTime + + return nil +} + //----------------------------------------------------------------------------- // Periodic Vesting Account @@ -485,6 +577,49 @@ func (pva PeriodicVestingAccount) MarshalYAML() (interface{}, error) { return string(bz), err } +func (pva PeriodicVestingAccount) MarshalJSONPB(m *jsonpb.Marshaler) ([]byte, error) { + bz, err := pva.BaseVestingAccount.BaseAccount.MarshalJSONPB(m) + if err != nil { + return nil, err + } + alias := vestingAccountJSON{ + BaseAccount: bz, + OriginalVesting: pva.BaseVestingAccount.OriginalVesting, + DelegatedFree: pva.BaseVestingAccount.DelegatedFree, + DelegatedVesting: pva.BaseVestingAccount.DelegatedVesting, + EndTime: pva.BaseVestingAccount.EndTime, + StartTime: pva.StartTime, + VestingPeriods: pva.VestingPeriods, + } + + return json.Marshal(alias) +} + +func (pva *PeriodicVestingAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz []byte) error { + var va vestingAccountJSON + + err := json.Unmarshal(bz, &va) + if err != nil { + return err + } + + ba := new(authtypes.BaseAccount) + if err := m.Unmarshal(strings.NewReader(string(va.BaseAccount)), ba); err != nil { + return err + } + pva.BaseVestingAccount = &BaseVestingAccount{ + BaseAccount: ba, + OriginalVesting: va.OriginalVesting, + DelegatedFree: va.DelegatedFree, + DelegatedVesting: va.DelegatedVesting, + EndTime: va.EndTime, + } + pva.StartTime = va.StartTime + pva.VestingPeriods = va.VestingPeriods + + return nil +} + //----------------------------------------------------------------------------- // Delayed Vesting Account @@ -551,3 +686,42 @@ func (dva DelayedVestingAccount) String() string { out, _ := dva.MarshalYAML() return out.(string) } + +func (dva DelayedVestingAccount) MarshalJSONPB(m *jsonpb.Marshaler) ([]byte, error) { + bz, err := dva.BaseAccount.MarshalJSONPB(m) + if err != nil { + return nil, err + } + alias := vestingAccountJSON{ + BaseAccount: bz, + OriginalVesting: dva.BaseVestingAccount.OriginalVesting, + DelegatedFree: dva.BaseVestingAccount.DelegatedFree, + DelegatedVesting: dva.BaseVestingAccount.DelegatedVesting, + EndTime: dva.BaseVestingAccount.EndTime, + } + + return json.Marshal(alias) +} + +func (dva *DelayedVestingAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz []byte) error { + var va vestingAccountJSON + + err := json.Unmarshal(bz, &va) + if err != nil { + return err + } + + ba := new(authtypes.BaseAccount) + if err := m.Unmarshal(strings.NewReader(string(va.BaseAccount)), ba); err != nil { + return err + } + dva.BaseVestingAccount = &BaseVestingAccount{ + BaseAccount: ba, + OriginalVesting: va.OriginalVesting, + DelegatedFree: va.DelegatedFree, + DelegatedVesting: va.DelegatedVesting, + EndTime: va.EndTime, + } + + return nil +} From c6bde73fcdcb3a05798c9c480179105e5f23772d Mon Sep 17 00:00:00 2001 From: egonspace Date: Thu, 16 Sep 2021 21:09:01 +0900 Subject: [PATCH 04/16] fix: BaseAccount marshal/unmarshal --- x/auth/types/account.go | 62 ++++++++++++------------- x/auth/types/account_test.go | 17 +++++++ x/auth/vesting/types/vesting_account.go | 37 +++++++-------- 3 files changed, 64 insertions(+), 52 deletions(-) diff --git a/x/auth/types/account.go b/x/auth/types/account.go index ed529cf4e3..214e9735f4 100644 --- a/x/auth/types/account.go +++ b/x/auth/types/account.go @@ -31,6 +31,10 @@ var ( BaseAccountSig = []byte("bacc") ModuleAccountSig = []byte("macc") + + PubKeyTypeSecp256k1 = byte(1) + PubKeyTypeEd25519 = byte(2) + PubKeyTypeMultisig = byte(3) ) // NewBaseAccount creates a new BaseAccount object @@ -408,9 +412,10 @@ type GenesisAccount interface { // custom json marshaler for BaseAccount & ModuleAccount type BaseAccountJSON struct { - Address string `json:"address"` - PubKey json.RawMessage `json:"pub_key"` - Sequence string `json:"sequence"` + Address string `json:"address"` + PubType byte `json:"pub_type"` + PubKey []byte `json:"pub_key"` + Sequence string `json:"sequence"` } func (acc BaseAccount) MarshalJSONPB(m *jsonpb.Marshaler) ([]byte, error) { @@ -420,14 +425,17 @@ func (acc BaseAccount) MarshalJSONPB(m *jsonpb.Marshaler) ([]byte, error) { bi.Sequence = strconv.FormatUint(acc.Sequence, 10) var bz []byte var err error - if acc.Ed25519PubKey != nil { - bz, err = codec.ProtoMarshalJSON(acc.Ed25519PubKey, m.AnyResolver) - } if acc.Secp256K1PubKey != nil { - bz, err = codec.ProtoMarshalJSON(acc.Secp256K1PubKey, m.AnyResolver) + bi.PubType = PubKeyTypeSecp256k1 + bz, err = acc.Secp256K1PubKey.Marshal() + } + if acc.Ed25519PubKey != nil { + bi.PubType = PubKeyTypeEd25519 + bz, err = acc.Ed25519PubKey.Marshal() } if acc.MultisigPubKey != nil { - bz, err = codec.ProtoMarshalJSON(acc.MultisigPubKey, m.AnyResolver) + bi.PubType = PubKeyTypeMultisig + bz, err = acc.MultisigPubKey.Marshal() } if err != nil { return nil, err @@ -443,12 +451,6 @@ func (acc *BaseAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz []byte) error if err != nil { return err } - /* TODO: do we need to validate address format here - err = sdk.ValidateAccAddress(bi.Address) - if err != nil { - return err - } - */ acc.Address = bi.Address acc.Sequence, err = strconv.ParseUint(bi.Sequence, 10, 64) @@ -456,29 +458,25 @@ func (acc *BaseAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz []byte) error return err } - done := false - if !done { + switch bi.PubType { + case PubKeyTypeSecp256k1: pk := new(secp256k1.PubKey) - any, _ := codectypes.NewAnyWithValue(pk) - if m.Unmarshal(strings.NewReader(string(bi.PubKey)), any) == nil { - acc.SetPubKey(pk) - done = true + if err := pk.Unmarshal(bi.PubKey); err != nil { + return err } - } - if !done { - pk := new(ed25519.PubKey) - any, _ := codectypes.NewAnyWithValue(pk) - if m.Unmarshal(strings.NewReader(string(bi.PubKey)), any) == nil { - acc.SetPubKey(pk) - done = true + acc.SetPubKey(pk) + case PubKeyTypeEd25519: + pk := new(secp256k1.PubKey) + if err := pk.Unmarshal(bi.PubKey); err != nil { + return err } - } - if !done { + acc.SetPubKey(pk) + case PubKeyTypeMultisig: pk := new(multisig.LegacyAminoPubKey) - any, _ := codectypes.NewAnyWithValue(pk) - if m.Unmarshal(strings.NewReader(string(bi.PubKey)), any) == nil { - acc.SetPubKey(pk) + if err := pk.Unmarshal(bi.PubKey); err != nil { + return err } + acc.SetPubKey(pk) } return nil } diff --git a/x/auth/types/account_test.go b/x/auth/types/account_test.go index 8cc5dd7db5..0a2ecd76cd 100644 --- a/x/auth/types/account_test.go +++ b/x/auth/types/account_test.go @@ -6,6 +6,8 @@ import ( "fmt" "testing" + "github.com/line/lbm-sdk/codec" + types2 "github.com/line/lbm-sdk/codec/types" "github.com/stretchr/testify/require" yaml "gopkg.in/yaml.v2" @@ -48,6 +50,21 @@ func TestBaseAddressPubKey(t *testing.T) { require.EqualValues(t, addr2, acc2.GetAddress()) } +func TestBaseAccountMarshalUnmarshalJSON(t *testing.T) { + interfaceRegistry := types2.NewInterfaceRegistry() + + cdc := codec.NewProtoCodec(interfaceRegistry) + _, pub, addr := testdata.KeyTestPubAddr() + acc := types.NewBaseAccountWithAddress(addr) + acc.SetPubKey(pub) + + bz := cdc.MustMarshalJSON(acc) + var acc2 types.BaseAccount + + cdc.MustUnmarshalJSON(bz, &acc2) + require.Equal(t, acc, &acc2) +} + func TestBaseSequence(t *testing.T) { _, _, addr := testdata.KeyTestPubAddr() acc := types.NewBaseAccountWithAddress(addr) diff --git a/x/auth/vesting/types/vesting_account.go b/x/auth/vesting/types/vesting_account.go index 47f10f4ea8..4331c5460b 100644 --- a/x/auth/vesting/types/vesting_account.go +++ b/x/auth/vesting/types/vesting_account.go @@ -3,7 +3,6 @@ package types import ( "encoding/json" "errors" - "strings" "time" "github.com/gogo/protobuf/jsonpb" @@ -247,15 +246,15 @@ func (bva *BaseVestingAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz []byte) return err } - ba := new(authtypes.BaseAccount) - if err := m.Unmarshal(strings.NewReader(string(va.BaseAccount)), ba); err != nil { + var ba authtypes.BaseAccount + if err := (&ba).UnmarshalJSONPB(m, va.BaseAccount); err != nil { return err } - bva.BaseAccount = ba + bva.BaseAccount = &ba bva.OriginalVesting = va.OriginalVesting - bva.DelegatedFree = va.DelegatedFree + bva.DelegatedFree = va.DelegatedFree bva.OriginalVesting = va.DelegatedVesting - bva.EndTime = va.EndTime + bva.EndTime = va.EndTime return nil } @@ -396,7 +395,6 @@ func (cva ContinuousVestingAccount) MarshalJSONPB(m *jsonpb.Marshaler) ([]byte, EndTime: cva.BaseVestingAccount.EndTime, StartTime: cva.StartTime, } - return json.Marshal(alias) } @@ -408,19 +406,18 @@ func (cva *ContinuousVestingAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz [ return err } - ba := new(authtypes.BaseAccount) - if err := m.Unmarshal(strings.NewReader(string(va.BaseAccount)), ba); err != nil { + var ba authtypes.BaseAccount + if err := (&ba).UnmarshalJSONPB(m, va.BaseAccount); err != nil { return err } cva.BaseVestingAccount = &BaseVestingAccount{ - BaseAccount: ba, + BaseAccount: &ba, OriginalVesting: va.OriginalVesting, DelegatedFree: va.DelegatedFree, DelegatedVesting: va.DelegatedVesting, EndTime: va.EndTime, } - cva.StartTime = va.StartTime - + cva.StartTime = va.StartTime return nil } @@ -603,19 +600,19 @@ func (pva *PeriodicVestingAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz []b return err } - ba := new(authtypes.BaseAccount) - if err := m.Unmarshal(strings.NewReader(string(va.BaseAccount)), ba); err != nil { + var ba authtypes.BaseAccount + if err := (&ba).UnmarshalJSONPB(m, va.BaseAccount); err != nil { return err } pva.BaseVestingAccount = &BaseVestingAccount{ - BaseAccount: ba, + BaseAccount: &ba, OriginalVesting: va.OriginalVesting, DelegatedFree: va.DelegatedFree, DelegatedVesting: va.DelegatedVesting, EndTime: va.EndTime, } - pva.StartTime = va.StartTime - pva.VestingPeriods = va.VestingPeriods + pva.StartTime = va.StartTime + pva.VestingPeriods = va.VestingPeriods return nil } @@ -711,12 +708,12 @@ func (dva *DelayedVestingAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz []by return err } - ba := new(authtypes.BaseAccount) - if err := m.Unmarshal(strings.NewReader(string(va.BaseAccount)), ba); err != nil { + var ba authtypes.BaseAccount + if err := (&ba).UnmarshalJSONPB(m, va.BaseAccount); err != nil { return err } dva.BaseVestingAccount = &BaseVestingAccount{ - BaseAccount: ba, + BaseAccount: &ba, OriginalVesting: va.OriginalVesting, DelegatedFree: va.DelegatedFree, DelegatedVesting: va.DelegatedVesting, From ffa950db49f3dff23961e8bc499642e454e671eb Mon Sep 17 00:00:00 2001 From: egonspace Date: Fri, 17 Sep 2021 18:36:41 +0900 Subject: [PATCH 05/16] fix: remove param store cache, fix consensus param --- baseapp/baseapp.go | 2 +- x/params/types/subspace.go | 79 --------------------------------- x/params/types/subspace_test.go | 49 -------------------- x/params/types/table.go | 2 - 4 files changed, 1 insertion(+), 131 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index c45d4f3bfe..f042ec6f0b 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -361,7 +361,7 @@ func (app *BaseApp) setCheckState(header ocproto.Header) { app.checkState = &state{ ms: ms, - ctx: ctx.WithConsensusParams(app.GetConsensusParams(ctx)), + ctx: ctx, } } diff --git a/x/params/types/subspace.go b/x/params/types/subspace.go index a37518fd66..0cea9f8823 100644 --- a/x/params/types/subspace.go +++ b/x/params/types/subspace.go @@ -3,8 +3,6 @@ package types import ( "fmt" "reflect" - "sync/atomic" - "unsafe" "github.com/line/lbm-sdk/codec" "github.com/line/lbm-sdk/store/prefix" @@ -91,27 +89,18 @@ func (s *Subspace) Validate(ctx sdk.Context, key []byte, value interface{}) erro func (s *Subspace) Get(ctx sdk.Context, key []byte, ptr interface{}) { s.checkType(key, ptr) - if s.loadFromCache(key, ptr) { - // cache hit - return - } store := s.kvStore(ctx) bz := store.Get(key) if err := s.legacyAmino.UnmarshalJSON(bz, ptr); err != nil { panic(err) } - s.cacheValue(key, ptr) } // GetIfExists queries for a parameter by key from the Subspace's KVStore and // sets the value to the provided pointer. If the value does not exist, it will // perform a no-op. func (s *Subspace) GetIfExists(ctx sdk.Context, key []byte, ptr interface{}) { - if s.loadFromCache(key, ptr) { - // cache hit - return - } store := s.kvStore(ctx) bz := store.Get(key) if bz == nil { @@ -123,7 +112,6 @@ func (s *Subspace) GetIfExists(ctx sdk.Context, key []byte, ptr interface{}) { if err := s.legacyAmino.UnmarshalJSON(bz, ptr); err != nil { panic(err) } - s.cacheValue(key, ptr) } // GetRaw queries for the raw values bytes for a parameter by key. @@ -134,9 +122,6 @@ func (s *Subspace) GetRaw(ctx sdk.Context, key []byte) []byte { // Has returns if a parameter key exists or not in the Subspace's KVStore. func (s *Subspace) Has(ctx sdk.Context, key []byte) bool { - if s.hasCache(key) { - return true - } store := s.kvStore(ctx) return store.Has(key) } @@ -159,69 +144,6 @@ func (s *Subspace) checkType(key []byte, value interface{}) { } } -// All the cache-related functions here are thread-safe. -// Currently, since `CheckTx` and `DeliverTx` can run without abci locking, -// these functions must be thread-safe as tx can run concurrently. -// The map data type is not thread-safe by itself, but concurrent access is -// possible with entry fixed. If we access the subspace with an unregistered key, -// it panics, ensuring that the entry of the map is not extended after the server runs. -// Value update and read operations for a single entry of a map can be performed concurrently by -// `atomic.StorePointer` and `atomic.LoadPointer`. -func (s *Subspace) cacheValue(key []byte, value interface{}) { - attr, ok := s.table.m[string(key)] - if !ok { - panic(fmt.Sprintf("parameter %s not registered", string(key))) - } - val := reflect.ValueOf(value) - if reflect.TypeOf(value).Kind() == reflect.Ptr { - val = val.Elem() - } - valueToBeCached := reflect.New(val.Type()) - valueToBeCached.Elem().Set(val) - atomic.StorePointer(&attr.cachedValue, unsafe.Pointer(&valueToBeCached)) -} - -func (s *Subspace) hasCache(key []byte) bool { - attr, ok := s.table.m[string(key)] - if !ok { - panic(fmt.Sprintf("parameter %s not registered", string(key))) - } - cachedValuePtr := (*reflect.Value)(atomic.LoadPointer(&attr.cachedValue)) - return cachedValuePtr != nil -} - -func (s *Subspace) loadFromCache(key []byte, value interface{}) bool { - attr, ok := s.table.m[string(key)] - if !ok { - return false - } - if reflect.TypeOf(value).Kind() != reflect.Ptr { - panic("value should be a Pointer") - } - - cachedValuePtr := (*reflect.Value)(atomic.LoadPointer(&attr.cachedValue)) - if cachedValuePtr == nil { - return false - } - reflect.ValueOf(value).Elem().Set((*cachedValuePtr).Elem()) - return true -} - -// Only for test -func (s *Subspace) GetCachedValueForTesting(key []byte, value interface{}) bool { - return s.loadFromCache(key, value) -} - -// Only for test -func (s *Subspace) HasCacheForTesting(key []byte) bool { - return s.hasCache(key) -} - -// Only for test -func (s *Subspace) SetCacheForTesting(key []byte, value interface{}) { - s.cacheValue(key, value) -} - // Set stores a value for given a parameter key assuming the parameter type has // been registered. It will panic if the parameter type has not been registered // or if the value cannot be encoded. A change record is also set in the Subspace's @@ -236,7 +158,6 @@ func (s *Subspace) Set(ctx sdk.Context, key []byte, value interface{}) { } store.Set(key, bz) - s.cacheValue(key, value) } // Update stores an updated raw value for a given parameter key assuming the diff --git a/x/params/types/subspace_test.go b/x/params/types/subspace_test.go index 43519d9941..15fc26f1dc 100644 --- a/x/params/types/subspace_test.go +++ b/x/params/types/subspace_test.go @@ -187,55 +187,6 @@ func (suite *SubspaceTestSuite) TestSetParamSet() { suite.Require().Equal(a.BondDenom, b.BondDenom) } -func (suite *SubspaceTestSuite) TestParamSetCache() { - a := params{ - UnbondingTime: time.Hour * 48, - MaxValidators: 100, - BondDenom: "stake", - } - b := params{} - // confirm cache is empty - suite.Require().False(suite.ss.GetCachedValueForTesting(keyUnbondingTime, &b.UnbondingTime)) - suite.Require().False(suite.ss.GetCachedValueForTesting(keyMaxValidators, &b.MaxValidators)) - suite.Require().False(suite.ss.GetCachedValueForTesting(keyBondDenom, &b.BondDenom)) - suite.Require().False(suite.ss.HasCacheForTesting(keyUnbondingTime)) - suite.Require().False(suite.ss.HasCacheForTesting(keyMaxValidators)) - suite.Require().False(suite.ss.HasCacheForTesting(keyBondDenom)) - suite.Require().NotEqual(a, b) - - suite.Require().NotPanics(func() { - suite.ss.SetParamSet(suite.ctx, &a) - }) - // confirm cached - suite.Require().True(suite.ss.GetCachedValueForTesting(keyUnbondingTime, &b.UnbondingTime)) - suite.Require().True(suite.ss.GetCachedValueForTesting(keyMaxValidators, &b.MaxValidators)) - suite.Require().True(suite.ss.GetCachedValueForTesting(keyBondDenom, &b.BondDenom)) - suite.Require().Equal(a, b) - - // update local variable - a.UnbondingTime = time.Hour * 24 - a.MaxValidators = 50 - a.BondDenom = "link" - - // confirm the cache is not updated - c := params{} - suite.Require().True(suite.ss.GetCachedValueForTesting(keyUnbondingTime, &c.UnbondingTime)) - suite.Require().True(suite.ss.GetCachedValueForTesting(keyMaxValidators, &c.MaxValidators)) - suite.Require().True(suite.ss.GetCachedValueForTesting(keyBondDenom, &c.BondDenom)) - suite.Require().Equal(b, c) // cached value is not updated - - // update only cache - suite.ss.SetCacheForTesting(keyUnbondingTime, &a.UnbondingTime) - suite.ss.SetCacheForTesting(keyMaxValidators, &a.MaxValidators) - suite.ss.SetCacheForTesting(keyBondDenom, &a.BondDenom) - - // ensure GetParamSet to get value not from db but from cache - suite.Require().NotPanics(func() { - suite.ss.GetParamSet(suite.ctx, &c) - }) - suite.Require().Equal(a, c) -} - func (suite *SubspaceTestSuite) TestName() { suite.Require().Equal("testsubspace", suite.ss.Name()) } diff --git a/x/params/types/table.go b/x/params/types/table.go index 89416eab4c..0933cb8343 100644 --- a/x/params/types/table.go +++ b/x/params/types/table.go @@ -2,7 +2,6 @@ package types import ( "reflect" - "unsafe" sdk "github.com/line/lbm-sdk/types" ) @@ -10,7 +9,6 @@ import ( type attribute struct { ty reflect.Type vfn ValueValidatorFn - cachedValue unsafe.Pointer } // KeyTable subspaces appropriate type for each parameter key From 2b9bee426252f076c5a103a6031b5372c35771e3 Mon Sep 17 00:00:00 2001 From: egonspace Date: Thu, 23 Sep 2021 09:55:01 +0900 Subject: [PATCH 06/16] fix: distribution key bug --- x/distribution/types/keys.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/distribution/types/keys.go b/x/distribution/types/keys.go index db38a7150f..456dbc4389 100644 --- a/x/distribution/types/keys.go +++ b/x/distribution/types/keys.go @@ -100,9 +100,9 @@ func GetValidatorAccumulatedCommissionAddress(key []byte) (valAddr sdk.ValAddres // gets the height from a validator's slash event key func GetValidatorSlashEventAddressHeight(key []byte) (valAddr sdk.ValAddress, height uint64) { - addr := key[1 : len(key)-8] + addr := key[1 : len(key)-16] // 8 bytes height + 8 bytes period valAddr = sdk.ValAddress(addr) - b := key[len(key)-8:] // the next 8 bytes represent the height + b := key[len(key)-16:len(key)-8] // the next 8 bytes represent the height height = binary.BigEndian.Uint64(b) return } From 793f5910c6a619965cda3b514c311f233e32f6af Mon Sep 17 00:00:00 2001 From: egonspace Date: Thu, 23 Sep 2021 10:01:52 +0900 Subject: [PATCH 07/16] fix: lint errors --- x/distribution/types/keys.go | 2 +- x/params/types/table.go | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/x/distribution/types/keys.go b/x/distribution/types/keys.go index 456dbc4389..714521e694 100644 --- a/x/distribution/types/keys.go +++ b/x/distribution/types/keys.go @@ -102,7 +102,7 @@ func GetValidatorAccumulatedCommissionAddress(key []byte) (valAddr sdk.ValAddres func GetValidatorSlashEventAddressHeight(key []byte) (valAddr sdk.ValAddress, height uint64) { addr := key[1 : len(key)-16] // 8 bytes height + 8 bytes period valAddr = sdk.ValAddress(addr) - b := key[len(key)-16:len(key)-8] // the next 8 bytes represent the height + b := key[len(key)-16 : len(key)-8] // the next 8 bytes represent the height height = binary.BigEndian.Uint64(b) return } diff --git a/x/params/types/table.go b/x/params/types/table.go index 0933cb8343..226117d1bc 100644 --- a/x/params/types/table.go +++ b/x/params/types/table.go @@ -7,8 +7,8 @@ import ( ) type attribute struct { - ty reflect.Type - vfn ValueValidatorFn + ty reflect.Type + vfn ValueValidatorFn } // KeyTable subspaces appropriate type for each parameter key From 437d1cc9bd68842e5d264d12215a4d95db54ad7b Mon Sep 17 00:00:00 2001 From: egonspace Date: Thu, 23 Sep 2021 11:09:33 +0900 Subject: [PATCH 08/16] fix: modify random param --- x/auth/simulation/genesis.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x/auth/simulation/genesis.go b/x/auth/simulation/genesis.go index 48d36366da..a742d40dfe 100644 --- a/x/auth/simulation/genesis.go +++ b/x/auth/simulation/genesis.go @@ -89,7 +89,8 @@ func GenSigVerifyCostSECP256K1(r *rand.Rand) uint64 { } func GenValidSigBlockPeriod(r *rand.Rand) uint64 { - return uint64(simulation.RandIntBetween(r, 1, 1000)) + // We use valid sig block period greater than 100 for testing + return uint64(simulation.RandIntBetween(r, 100, 1000)) } // RandomizedGenState generates a random GenesisState for auth From cc190d440c3e42f3e0f1970f5f4ca0554e09c327 Mon Sep 17 00:00:00 2001 From: egonspace Date: Thu, 23 Sep 2021 11:38:20 +0900 Subject: [PATCH 09/16] test: disable cosmovisor test --- .github/workflows/test.yml | 43 +++++++++++++++++++------------------- 1 file changed, 22 insertions(+), 21 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index c98b6f9ba2..69ef77d67b 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -65,27 +65,28 @@ jobs: - name: Build run: GOOS=linux CGO_ENABLED=1 GOARCH=${{ matrix.goarch }} CC=${{ matrix.gcc }} LEDGER_ENABLED=false make build - test-cosmovisor: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - uses: actions/setup-go@v2.1.4 - with: - go-version: 1.15 - - name: Display go version - run: go version - - uses: technote-space/get-diff-action@v5.0.1 - id: git_diff - with: - PREFIX_FILTER: | - cosmovisor - PATTERNS: | - **/**.go - go.mod - go.sum - - name: Run cosmovisor tests - run: cd cosmovisor; make - if: env.GIT_DIFF +# TODO: disable test-cosmovisor; this test uses uploaded binary(cosmos-sdk) +# test-cosmovisor: +# runs-on: ubuntu-latest +# steps: +# - uses: actions/checkout@v2 +# - uses: actions/setup-go@v2.1.4 +# with: +# go-version: 1.15 +# - name: Display go version +# run: go version +# - uses: technote-space/get-diff-action@v5.0.1 +# id: git_diff +# with: +# PREFIX_FILTER: | +# cosmovisor +# PATTERNS: | +# **/**.go +# go.mod +# go.sum +# - name: Run cosmovisor tests +# run: cd cosmovisor; make +# if: env.GIT_DIFF split-test-files: runs-on: ubuntu-latest From 02e37c37acbeaa97efbc66944e33183de40c2201 Mon Sep 17 00:00:00 2001 From: egonspace Date: Thu, 23 Sep 2021 15:19:47 +0900 Subject: [PATCH 10/16] fix: address conversion bug --- x/distribution/keeper/hooks.go | 2 +- x/slashing/keeper/hooks.go | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/x/distribution/keeper/hooks.go b/x/distribution/keeper/hooks.go index 753f397d96..aa0702ea3d 100644 --- a/x/distribution/keeper/hooks.go +++ b/x/distribution/keeper/hooks.go @@ -43,7 +43,7 @@ func (h Hooks) AfterValidatorRemoved(ctx sdk.Context, _ sdk.ConsAddress, valAddr // add to validator account if !coins.IsZero() { - accAddr := sdk.AccAddress(valAddr) + accAddr := valAddr.ToAccAddress() withdrawAddr := h.k.GetDelegatorWithdrawAddr(ctx, accAddr) if err := h.k.bankKeeper.SendCoinsFromModuleToAccount(ctx, types.ModuleName, withdrawAddr, coins); err != nil { diff --git a/x/slashing/keeper/hooks.go b/x/slashing/keeper/hooks.go index d67a01211d..19020ae56b 100644 --- a/x/slashing/keeper/hooks.go +++ b/x/slashing/keeper/hooks.go @@ -3,8 +3,6 @@ package keeper import ( "time" - "github.com/line/ostracon/crypto" - sdk "github.com/line/lbm-sdk/types" "github.com/line/lbm-sdk/x/slashing/types" ) @@ -39,7 +37,8 @@ func (k Keeper) AfterValidatorCreated(ctx sdk.Context, valAddr sdk.ValAddress) e // AfterValidatorRemoved deletes the address-pubkey relation when a validator is removed, func (k Keeper) AfterValidatorRemoved(ctx sdk.Context, address sdk.ConsAddress) { - k.deleteAddrPubkeyRelation(ctx, crypto.Address(address)) + addrBytes, _ := sdk.ConsAddressToBytes(address.String()) + k.deleteAddrPubkeyRelation(ctx, addrBytes) } //_________________________________________________________________________________________ From 27e4b00e9827f6646fbbcaaaa322f8cf733ed9c7 Mon Sep 17 00:00:00 2001 From: egonspace Date: Fri, 24 Sep 2021 11:49:34 +0900 Subject: [PATCH 11/16] docs: fix markdown check failures --- docs/core/proto-docs.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/core/proto-docs.md b/docs/core/proto-docs.md index aea9ed50ce..2cc4f029a2 100644 --- a/docs/core/proto-docs.md +++ b/docs/core/proto-docs.md @@ -2070,7 +2070,7 @@ source tracing information path. ### FungibleTokenPacketData FungibleTokenPacketData defines a struct for the packet payload See FungibleTokenPacketData spec: -https://github.com/cosmos/ics/tree/master/spec/ics-020-fungible-token-transfer#data-structures +https://github.com/cosmos/ibc/tree/master/spec/app/ics-020-fungible-token-transfer#data-structures | Field | Type | Label | Description | @@ -2401,7 +2401,7 @@ Params defines the set of IBC light client parameters. ### MsgTransfer MsgTransfer defines a msg to transfer fungible tokens (i.e Coins) between ICS20 enabled chains. See ICS Spec here: -https://github.com/cosmos/ics/tree/master/spec/ics-020-fungible-token-transfer#data-structures +https://github.com/cosmos/ics/tree/master/spec/app/ics-020-fungible-token-transfer#data-structures | Field | Type | Label | Description | @@ -2464,7 +2464,7 @@ NOTE: The field numbers 21 and 22 were explicitly chosen to avoid accidental conflicts with other protobuf message formats used for acknowledgements. The first byte of any message with this format will be the non-ASCII values `0xaa` (result) or `0xb2` (error). Implemented as defined by ICS: -https://github.com/cosmos/ics/tree/master/spec/ics-004-channel-and-packet-semantics#acknowledgement-envelope +https://github.com/cosmos/ibc/tree/master/spec/core/ics-004-channel-and-packet-semantics#acknowledgement-envelope | Field | Type | Label | Description | From 3fab0fb9c9d8157b63f9c1f089d9bf1d4bacb9d8 Mon Sep 17 00:00:00 2001 From: egonspace Date: Fri, 24 Sep 2021 14:03:20 +0900 Subject: [PATCH 12/16] test: fix action script of sim test --- .github/workflows/sims.yml | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/.github/workflows/sims.yml b/.github/workflows/sims.yml index 24c62dfcb5..d1894a1e57 100644 --- a/.github/workflows/sims.yml +++ b/.github/workflows/sims.yml @@ -61,6 +61,7 @@ jobs: with: PATTERNS: | **/**.go + !**/**_test.go go.mod go.sum - uses: actions/cache@v2.1.3 @@ -85,8 +86,9 @@ jobs: run: go version - uses: technote-space/get-diff-action@v5.0.1 with: - SUFFIX_FILTER: | + PATTERNS: | **/**.go + !**/**_test.go go.mod go.sum SET_ENV_NAME_INSERTIONS: 1 @@ -113,8 +115,9 @@ jobs: run: go version - uses: technote-space/get-diff-action@v5.0.1 with: - SUFFIX_FILTER: | + PATTERNS: | **/**.go + !**/**_test.go go.mod go.sum SET_ENV_NAME_INSERTIONS: 1 @@ -141,8 +144,9 @@ jobs: run: go version - uses: technote-space/get-diff-action@v5.0.1 with: - SUFFIX_FILTER: | + PATTERNSR: | **/**.go + !**/**_test.go go.mod go.sum SET_ENV_NAME_INSERTIONS: 1 From 710c4dd36843a3c762e891a7daa9acfa8445aaaa Mon Sep 17 00:00:00 2001 From: egonspace Date: Fri, 24 Sep 2021 14:03:55 +0900 Subject: [PATCH 13/16] test: remove an env for private repo --- .github/workflows/sims.yml | 3 --- 1 file changed, 3 deletions(-) diff --git a/.github/workflows/sims.yml b/.github/workflows/sims.yml index d1894a1e57..b5daf0f839 100644 --- a/.github/workflows/sims.yml +++ b/.github/workflows/sims.yml @@ -1,9 +1,6 @@ name: Sims # Sims workflow runs multiple types of simulations (nondeterminism, import-export, after-import, multi-seed-short) # This workflow will run on all Pull Requests, if a .go, .mod or .sum file have been changed -env: - GOPRIVATE: "github.com/line/*" - on: pull_request: push: From 1a7149d555fb175e562f32c80846a34fabd83629 Mon Sep 17 00:00:00 2001 From: egonspace Date: Fri, 24 Sep 2021 20:23:15 +0900 Subject: [PATCH 14/16] test: fix action script typo --- .github/workflows/sims.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/sims.yml b/.github/workflows/sims.yml index b5daf0f839..2c904ea224 100644 --- a/.github/workflows/sims.yml +++ b/.github/workflows/sims.yml @@ -141,7 +141,7 @@ jobs: run: go version - uses: technote-space/get-diff-action@v5.0.1 with: - PATTERNSR: | + PATTERNS: | **/**.go !**/**_test.go go.mod From c9d2940ee6f23d1ee4f1e120fe75616a30baaadc Mon Sep 17 00:00:00 2001 From: egonspace Date: Fri, 24 Sep 2021 21:09:54 +0900 Subject: [PATCH 15/16] fix: apply comment --- x/auth/types/account.go | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/x/auth/types/account.go b/x/auth/types/account.go index 214e9735f4..d29e47f289 100644 --- a/x/auth/types/account.go +++ b/x/auth/types/account.go @@ -411,11 +411,14 @@ type GenesisAccount interface { // custom json marshaler for BaseAccount & ModuleAccount +type PubKeyJSON struct { + Type byte `json:"type"` + Key []byte `json:"key"` +} type BaseAccountJSON struct { - Address string `json:"address"` - PubType byte `json:"pub_type"` - PubKey []byte `json:"pub_key"` - Sequence string `json:"sequence"` + Address string `json:"address"` + PubKey PubKeyJSON `json:"pub_key"` + Sequence string `json:"sequence"` } func (acc BaseAccount) MarshalJSONPB(m *jsonpb.Marshaler) ([]byte, error) { @@ -426,21 +429,21 @@ func (acc BaseAccount) MarshalJSONPB(m *jsonpb.Marshaler) ([]byte, error) { var bz []byte var err error if acc.Secp256K1PubKey != nil { - bi.PubType = PubKeyTypeSecp256k1 + bi.PubKey.Type = PubKeyTypeSecp256k1 bz, err = acc.Secp256K1PubKey.Marshal() } if acc.Ed25519PubKey != nil { - bi.PubType = PubKeyTypeEd25519 + bi.PubKey.Type = PubKeyTypeEd25519 bz, err = acc.Ed25519PubKey.Marshal() } if acc.MultisigPubKey != nil { - bi.PubType = PubKeyTypeMultisig + bi.PubKey.Type = PubKeyTypeMultisig bz, err = acc.MultisigPubKey.Marshal() } if err != nil { return nil, err } - bi.PubKey = bz + bi.PubKey.Key = bz return json.Marshal(bi) } @@ -458,22 +461,22 @@ func (acc *BaseAccount) UnmarshalJSONPB(m *jsonpb.Unmarshaler, bz []byte) error return err } - switch bi.PubType { + switch bi.PubKey.Type { case PubKeyTypeSecp256k1: pk := new(secp256k1.PubKey) - if err := pk.Unmarshal(bi.PubKey); err != nil { + if err := pk.Unmarshal(bi.PubKey.Key); err != nil { return err } acc.SetPubKey(pk) case PubKeyTypeEd25519: pk := new(secp256k1.PubKey) - if err := pk.Unmarshal(bi.PubKey); err != nil { + if err := pk.Unmarshal(bi.PubKey.Key); err != nil { return err } acc.SetPubKey(pk) case PubKeyTypeMultisig: pk := new(multisig.LegacyAminoPubKey) - if err := pk.Unmarshal(bi.PubKey); err != nil { + if err := pk.Unmarshal(bi.PubKey.Key); err != nil { return err } acc.SetPubKey(pk) From 3509accba212fd8ccbe4a31c21e256b3847b33a6 Mon Sep 17 00:00:00 2001 From: egonspace Date: Fri, 24 Sep 2021 21:14:38 +0900 Subject: [PATCH 16/16] chore: fix lint error --- x/auth/types/account.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/x/auth/types/account.go b/x/auth/types/account.go index d29e47f289..bf071aceda 100644 --- a/x/auth/types/account.go +++ b/x/auth/types/account.go @@ -412,8 +412,8 @@ type GenesisAccount interface { // custom json marshaler for BaseAccount & ModuleAccount type PubKeyJSON struct { - Type byte `json:"type"` - Key []byte `json:"key"` + Type byte `json:"type"` + Key []byte `json:"key"` } type BaseAccountJSON struct { Address string `json:"address"`