Closed kfangw closed 3 years ago
Merging #916 into master will decrease coverage by
0.59%
. The diff coverage is42.97%
.
@@ Coverage Diff @@
## master #916 +/- ##
=========================================
- Coverage 60.47% 59.88% -0.6%
=========================================
Files 159 162 +3
Lines 11203 11452 +249
=========================================
+ Hits 6775 6858 +83
- Misses 3629 3792 +163
- Partials 799 802 +3
Flag | Coverage Δ | |
---|---|---|
#integration_tests_long_term | 43.9% <ø> (ø) |
:arrow_up: |
#integration_tests_node | 39.89% <ø> (-0.68%) |
:arrow_down: |
#integration_tests_retry | 37.21% <ø> (+0.05%) |
:arrow_up: |
#unittests | 47.57% <42.97%> (-0.11%) |
:arrow_down: |
Impacted Files | Coverage Δ | |
---|---|---|
lib/statedb/store/iavlstore.go | 37.5% <37.5%> (ø) |
|
lib/statedb/stateobject.go | 39.6% <39.6%> (ø) |
|
lib/statedb/statedb.go | 47.41% <47.41%> (ø) |
|
lib/network/http2_client.go | 54.63% <0%> (-10.31%) |
:arrow_down: |
lib/node/runner/node_api/api_node_item.go | 53.84% <0%> (-10.26%) |
:arrow_down: |
lib/block/transaction_pool.go | 52.83% <0%> (-3.78%) |
:arrow_down: |
lib/node/runner/util.go | 71.42% <0%> (-3.58%) |
:arrow_down: |
lib/node/runner/finish_ballot.go | 43.86% <0%> (-1.42%) |
:arrow_down: |
lib/node/runner/checker.go | 73.36% <0%> (-0.23%) |
:arrow_down: |
... and 2 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact)
,ø = not affected
,? = missing data
Powered by Codecov. Last update c71cd9b...5b2077b. Read the comment docs.
How about that we make a branch for it? and make a pr of this branch (for master rebasing) :->
@anarcher good idea. However, as you can see, this is in WIP. When I finalize this feature (especially test cases) than I will make a branch what you want.
IMHO, state.DB
and StateDB
are confusing by same naming DB
. I would like have layering naming. e.g. State
-(use)-> Store
-(use)-> DB(leveldb,rocksdb)
.
I am not clear that we have account (only) based states? As you know, Cosmos style is that multi-substores. With this approaches, we can have accounts and contracts and storages each other. (not having account.Code and account.Storages).
Block.StateRoot = MultiStore's Roothash and each of substores can be accounts(one of StoreKVKey) and contracts, storages... (staking or gov...)
I am not clear that we have account (only) based states? As you know, Cosmos style is that multi-substores. With this approaches, we can have accounts and contracts and storages each other. (not having account.Code and account.Storages).
Block.StateRoot = MultiStore's Roothash and each of substores can be accounts(one of StoreKVKey) and contracts, storages... (staking or gov...)
@anarcher Good idea. Balance, Code, and Storage can be divided into substores. But, for the Storage, each account can hold multiple (k,v) pairs. The key may duplicated To solve this, IMO, PrefixDB, PrefixStore may be adaptable.
/balance/(address, balance) pair for a data item /code/(address, code) pair /storage/(addressprefixed data key, value) pair
Is it makes sense?
This is a example code snippet
func TestMulti(t *testing.T) {
db := dbm.NewMemDB()
st := store.NewCommitMultiStore(db)
key1 := sdk.NewKVStoreKey("balance")
key2 := sdk.NewKVStoreKey("code")
key3 := sdk.NewKVStoreKey("storage")
st.MountStoreWithDB(key1, sdk.StoreTypeIAVL, nil)
st.MountStoreWithDB(key2, sdk.StoreTypeIAVL, nil)
st.MountStoreWithDB(key3, sdk.StoreTypeIAVL, nil)
err := st.LoadLatestVersion()
require.Nil(t, err)
addrExample := []byte{0x00, 0x00, 0x01, 0x02, 0xFF}
balanceStore := st.GetCommitKVStore(key1)
codeStore := st.GetCommitKVStore(key2)
storageStore := st.GetCommitKVStore(key3)
theAccountStorageStore := storageStore.Prefix(addrExample)
balanceStore.Set(addrExample, []byte("1000"))
codeStore.Set(addrExample, []byte("print hello world"))
theAccountStorageStore.Set([]byte("variableKey"), []byte("variableValue"))
commitID := st.Commit()
t.Log(commitID.Hash) //Hash for root Store
t.Log(balanceStore.LastCommitID().Hash)
t.Log(codeStore.LastCommitID().Hash)
t.Log(storageStore.LastCommitID().Hash)
}
My first about storage is that storage treated as []byte from storage type. ( accAddress -> storage bytes )
It means that we storage items (k/v) saved one byte array in store (with amino) So we don’t need to prefix if it cares like this. But l am not sure about it. We need to have times to consider it :-)
Github Issue
Background
Solution
Possible Drawbacks