Problem: memiavl don't support `CacheMultiStoreWithVersion` #1114

yihuang · 2023-07-28T02:45:40Z

CacheMultiStoreWithVersion is needed for grpc query service and cosmwasm snapshotting.

👮🏻👮🏻👮🏻 !!!! REFERENCE THE PROBLEM YOUR ARE SOLVING IN THE PR TITLE AND DESCRIBE YOUR SOLUTION HERE !!!! DO NOT FORGET !!!! 👮🏻👮🏻👮🏻

PR Checklist:

Thank you for your code, it's appreciated! :)

`CacheMultiStoreWithVersion` is needed for grpc query service and cosmwasm snapshotting.

CHANGELOG.md

Signed-off-by: yihuang <huang@crypto.com>

store/rootmulti/store.go

+	for k, store := range rs.stores {
+		if store.GetStoreType() != types.StoreTypeIAVL {
+			stores[k] = store
+		}
+	}


store/rootmulti/store.go

+		return rs.CacheMultiStore(), nil
+	}
+	opts := rs.opts
+	opts.TargetVersion = uint32(version)


store/rootmulti/store.go

+	for k, store := range rs.stores {
+		if store.GetStoreType() != types.StoreTypeIAVL {
+			stores[k] = store
+		}
+	}


codecov · 2023-07-28T03:18:38Z

Codecov Report

Merging #1114 (cdc8f69) into main (082feca) will decrease coverage by 0.03%.
The diff coverage is 80.64%.

@@            Coverage Diff             @@
##             main    #1114      +/-   ##
==========================================
- Coverage   46.16%   46.13%   -0.03%     
==========================================
  Files          82       82              
  Lines        7454     7456       +2     
==========================================
- Hits         3441     3440       -1     
- Misses       3648     3651       +3     
  Partials      365      365

Files Changed	Coverage Δ
memiavl/multitree.go	`72.02% <79.31%> (-0.51%)`	⬇️
memiavl/export.go	`58.42% <100.00%> (ø)`

... and 1 file with indirect coverage changes

store/rootmulti/store.go

yzang2019

Thanks!

yzang2019 · 2023-07-31T14:56:44Z

@yihuang After integrating this change with our chain, we are seeing some concurrent map writes error when exporting snapshot:

fatal error: concurrent map writes
goroutine 2889 [running]:
github.com/cosmos/iavl/cache.(*lruCache).Add(0xc0003f6240, {0x2a9fd40, 0xc004ae4630})
	/root/go/pkg/mod/github.com/sei-protocol/sei-iavl@v0.1.8-0.20230726213826-031d03d26f2d/cache/cache.go:73 +0x2d0
github.com/sei-protocol/mmap-iavl/memiavl.(*Tree).Get(0xc00052c140, {0xc0039431c0, 0x19, 0x19})
	/root/go/pkg/mod/github.com/sei-protocol/mmap-iavl@v1.0.4/memiavl/tree.go:215 +0x13b
github.com/sei-protocol/mmap-iavl/store/memiavlstore.(*Store).Get(0x41b223?, {0xc0039431c0?, 0xc0031c3580?, 0x20?})
	/root/go/pkg/mod/github.com/sei-protocol/mmap-iavl@v1.0.4/store/memiavlstore/store.go:99 +0x25
github.com/cosmos/cosmos-sdk/store/cachekv.(*Store).Get(0xc00055ad80, {0xc0039431c0, 0x19, 0x19})
	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/store/cachekv/store.go:116 +0x14b
github.com/cosmos/cosmos-sdk/store/cachekv.(*Store).Get(0xc003dbe720, {0xc0039431c0, 0x19, 0x19})
	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/store/cachekv/store.go:116 +0x14b
github.com/cosmos/cosmos-sdk/store/gaskv.(*Store).Get(0xc005296120, {0xc0039431c0, 0x19, 0x19})
	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/store/gaskv/store.go:43 +0x71
github.com/cosmos/cosmos-sdk/store/prefix.Store.Get({{0x2accf10, 0xc005296120}, {0xc0002a9340, 0x4, 0x20}}, {0x3b18b80, 0x15, 0x3?})
	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/store/prefix/store.go:72 +0x10c
github.com/cosmos/cosmos-sdk/x/params/types.Subspace.Get({{0x2accdc0, 0xc0001b4c10}, 0xc0003fe3a8, {0x2aa5960, 0xc0006995a0}, {0x2aa59b0, 0xc000699670}, {0xc0002a9340, 0x3, 0x20}, ...}, ...)
	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/x/params/types/subspace.go:106 +0x213
github.com/cosmos/cosmos-sdk/x/params/types.Subspace.GetParamSet({{0x2accdc0, 0xc0001b4c10}, 0xc0003fe3a8, {0x2aa5960, 0xc0006995a0}, {0x2aa59b0, 0xc000699670}, {0xc0002a9340, 0x3, 0x20}, ...}, ...)
	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/x/params/types/subspace.go:222 +0x105
github.com/sei-protocol/sei-chain/x/dex/keeper.Keeper.GetParams(...)
	/sei-protocol/sei-chain/x/dex/keeper/params.go:11

Are you using a specific cosmos iavl version that has the concurrent write support?

yihuang · 2023-08-01T00:43:44Z

@yihuang After integrating this change with our chain, we are seeing some concurrent map writes error when exporting snapshot:


fatal error: concurrent map writes

goroutine 2889 [running]:

github.com/cosmos/iavl/cache.(*lruCache).Add(0xc0003f6240, {0x2a9fd40, 0xc004ae4630})

	/root/go/pkg/mod/github.com/sei-protocol/sei-iavl@v0.1.8-0.20230726213826-031d03d26f2d/cache/cache.go:73 +0x2d0

github.com/sei-protocol/mmap-iavl/memiavl.(*Tree).Get(0xc00052c140, {0xc0039431c0, 0x19, 0x19})

	/root/go/pkg/mod/github.com/sei-protocol/mmap-iavl@v1.0.4/memiavl/tree.go:215 +0x13b

github.com/sei-protocol/mmap-iavl/store/memiavlstore.(*Store).Get(0x41b223?, {0xc0039431c0?, 0xc0031c3580?, 0x20?})

	/root/go/pkg/mod/github.com/sei-protocol/mmap-iavl@v1.0.4/store/memiavlstore/store.go:99 +0x25

github.com/cosmos/cosmos-sdk/store/cachekv.(*Store).Get(0xc00055ad80, {0xc0039431c0, 0x19, 0x19})

	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/store/cachekv/store.go:116 +0x14b

github.com/cosmos/cosmos-sdk/store/cachekv.(*Store).Get(0xc003dbe720, {0xc0039431c0, 0x19, 0x19})

	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/store/cachekv/store.go:116 +0x14b

github.com/cosmos/cosmos-sdk/store/gaskv.(*Store).Get(0xc005296120, {0xc0039431c0, 0x19, 0x19})

	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/store/gaskv/store.go:43 +0x71

github.com/cosmos/cosmos-sdk/store/prefix.Store.Get({{0x2accf10, 0xc005296120}, {0xc0002a9340, 0x4, 0x20}}, {0x3b18b80, 0x15, 0x3?})

	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/store/prefix/store.go:72 +0x10c

github.com/cosmos/cosmos-sdk/x/params/types.Subspace.Get({{0x2accdc0, 0xc0001b4c10}, 0xc0003fe3a8, {0x2aa5960, 0xc0006995a0}, {0x2aa59b0, 0xc000699670}, {0xc0002a9340, 0x3, 0x20}, ...}, ...)

	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/x/params/types/subspace.go:106 +0x213

github.com/cosmos/cosmos-sdk/x/params/types.Subspace.GetParamSet({{0x2accdc0, 0xc0001b4c10}, 0xc0003fe3a8, {0x2aa5960, 0xc0006995a0}, {0x2aa59b0, 0xc000699670}, {0xc0002a9340, 0x3, 0x20}, ...}, ...)

	/root/go/pkg/mod/github.com/sei-protocol/sei-cosmos@v0.2.61-0.20230727213234-a9730a9620a2/x/params/types/subspace.go:222 +0x105

github.com/sei-protocol/sei-chain/x/dex/keeper.Keeper.GetParams(...)

	/sei-protocol/sei-chain/x/dex/keeper/params.go:11

Are you using a specific cosmos iavl version that has the concurrent write support?

There's a Copy method called before doing snapshotting, there should have no concurrency issue, don't you call snapshot from sdk? Or do you call directly?

Signed-off-by: yihuang <huang@crypto.com>

Problem: memiavl don't support CacheMultiStoreWithVersion

6eaa617

`CacheMultiStoreWithVersion` is needed for grpc query service and cosmwasm snapshotting.

yihuang requested a review from a team as a code owner July 28, 2023 02:45

yihuang requested review from mmsqe and leejw51crypto and removed request for a team July 28, 2023 02:45

yihuang commented Jul 28, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

Update CHANGELOG.md

155d8f7

Signed-off-by: yihuang <huang@crypto.com>

github-advanced-security bot found potential problems Jul 28, 2023

View reviewed changes

store/rootmulti/store.go

Comment on lines +183 to +187

for k, store := range rs.stores {

if store.GetStoreType() != types.StoreTypeIAVL {

stores[k] = store

}

}

Check warning

Code scanning / CodeQL

Iteration over map

Iteration over map may be a possible source of non-determinism

mmsqe reviewed Jul 28, 2023

View reviewed changes

store/rootmulti/store.go Show resolved Hide resolved

mmsqe approved these changes Jul 28, 2023

View reviewed changes

yzang2019 reviewed Jul 28, 2023

View reviewed changes

store/rootmulti/store.go Show resolved Hide resolved

yzang2019 approved these changes Jul 31, 2023

View reviewed changes

Merge branch 'main' into branch-store

fc76659

Signed-off-by: yihuang <huang@crypto.com>

yihuang enabled auto-merge August 8, 2023 02:30

Merge branch 'main' into branch-store

cdc8f69

yihuang added this pull request to the merge queue Aug 9, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 9, 2023

mmsqe added this pull request to the merge queue Aug 9, 2023

github-merge-queue bot removed this pull request from the merge queue due to failed status checks Aug 9, 2023

yihuang added this pull request to the merge queue Aug 9, 2023

Merged via the queue into crypto-org-chain:main with commit af389f6 Aug 9, 2023

coderabbitai bot mentioned this pull request Apr 19, 2024

Problem: panic on invalid elasticity_multiplier #1397

Merged

13 tasks

This was referenced Apr 26, 2024

Problem: no end-to-end encryption module #1407

Merged

Problem: encryption-key cmd is not supported #1409

Merged

Problem: no keyring interface for e2ee to store arbitrary payload #1413

Merged

This was referenced Apr 29, 2024

Problem: no encrypt and decrypt cmds for message #1411

Merged

Problem: no efficient batch query for encryption keys #1415

Merged

Problem: new evm tx format not integrated #1414

Merged

Problem: no validate for malformed e2ee key in register #1421

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Problem: memiavl don't support `CacheMultiStoreWithVersion` #1114

Problem: memiavl don't support `CacheMultiStoreWithVersion` #1114

yihuang commented Jul 28, 2023

codecov bot commented Jul 28, 2023 •

edited

Loading

yzang2019 left a comment

yzang2019 commented Jul 31, 2023 •

edited

Loading

yihuang commented Aug 1, 2023

Problem: memiavl don't support CacheMultiStoreWithVersion #1114

Problem: memiavl don't support CacheMultiStoreWithVersion #1114

Conversation

yihuang commented Jul 28, 2023

PR Checklist:

codecov bot commented Jul 28, 2023 • edited Loading

Codecov Report

yzang2019 left a comment

Choose a reason for hiding this comment

yzang2019 commented Jul 31, 2023 • edited Loading

yihuang commented Aug 1, 2023

Problem: memiavl don't support `CacheMultiStoreWithVersion` #1114

Problem: memiavl don't support `CacheMultiStoreWithVersion` #1114

codecov bot commented Jul 28, 2023 •

edited

Loading

yzang2019 commented Jul 31, 2023 •

edited

Loading