Skip to content

Commit

Permalink
[occ] Fix parent store readset validation (#348)
Browse files Browse the repository at this point in the history
## Describe your changes and provide context
This fixes the validation to remove a panic for a case that can actually
occur if a transaction writes a key that is later read, and that writing
transaction is reverted and then the readset validation reads from
parent store. In this case, the readset would have a conflict based on
the data available in parent store, so we shouldn't panic. This also
adds in the resource types needed for the new DEX_MEM keys

## Testing performed to validate your change
Tested in loadtest cluster
  • Loading branch information
udpatil authored Nov 10, 2023
1 parent 931e2f6 commit eac8657
Show file tree
Hide file tree
Showing 5 changed files with 154 additions and 97 deletions.
4 changes: 4 additions & 0 deletions proto/cosmos/accesscontrol/constants.proto
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,11 @@ enum ResourceType {
KV_DEX_SHORT_ORDER_COUNT = 92; // child of KV_DEX

KV_BANK_DEFERRED = 93; // child of KV
reserved 94;
KV_BANK_DEFERRED_MODULE_TX_INDEX = 95; // child of KV_BANK_DEFERRED

KV_DEX_MEM_CONTRACTS_TO_PROCESS = 96; // child of KV_DEX_MEM
KV_DEX_MEM_DOWNSTREAM_CONTRACTS = 97; // child of KV_DEX_MEM
}

enum WasmMessageSubtype {
Expand Down
4 changes: 2 additions & 2 deletions store/multiversion/store.go
Original file line number Diff line number Diff line change
Expand Up @@ -345,10 +345,10 @@ func (s *Store) checkReadsetAtIndex(index int) (bool, []int) {
// get the latest value from the multiversion store
latestValue := s.GetLatestBeforeIndex(index, []byte(key))
if latestValue == nil {
// TODO: maybe we don't even do this check?
// this is possible if we previously read a value from a transaction write that was later reverted, so this time we read from parent store
parentVal := s.parentStore.Get([]byte(key))
if !bytes.Equal(parentVal, value) {
panic("there shouldn't be readset conflicts with parent kv store, since it shouldn't change")
valid = false
}
} else {
// if estimate, mark as conflict index - but don't invalidate
Expand Down
47 changes: 44 additions & 3 deletions store/multiversion/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -231,10 +231,51 @@ func TestMultiVersionStoreValidateState(t *testing.T) {
valid, conflicts = mvs.ValidateTransactionState(5)
require.False(t, valid)
require.Equal(t, []int{4}, conflicts)
}

func TestMultiVersionStoreParentValidationMismatch(t *testing.T) {
parentKVStore := dbadapter.Store{DB: dbm.NewMemDB()}
mvs := multiversion.NewMultiVersionStore(parentKVStore)

parentKVStore.Set([]byte("key2"), []byte("value0"))
parentKVStore.Set([]byte("key3"), []byte("value3"))
parentKVStore.Set([]byte("key4"), []byte("value4"))
parentKVStore.Set([]byte("key5"), []byte("value5"))

writeset := make(multiversion.WriteSet)
writeset["key1"] = []byte("value1")
writeset["key2"] = []byte("value2")
writeset["key3"] = nil
mvs.SetWriteset(1, 2, writeset)

// assert panic for parent store mismatch
parentKVStore.Set([]byte("key5"), []byte("value6"))
require.Panics(t, func() { mvs.ValidateTransactionState(5) })
readset := make(multiversion.ReadSet)
readset["key1"] = []byte("value1")
readset["key2"] = []byte("value2")
readset["key3"] = nil
readset["key4"] = []byte("value4")
readset["key5"] = []byte("value5")
mvs.SetReadset(5, readset)

// assert no readset is valid
valid, conflicts := mvs.ValidateTransactionState(4)
require.True(t, valid)
require.Empty(t, conflicts)

// assert readset index 5 is valid
valid, conflicts = mvs.ValidateTransactionState(5)
require.True(t, valid)
require.Empty(t, conflicts)

// overwrite tx writeset for tx1 - no longer writes key1
writeset2 := make(multiversion.WriteSet)
writeset2["key2"] = []byte("value2")
writeset2["key3"] = nil
mvs.SetWriteset(1, 3, writeset2)

// assert readset index 5 is invalid - because of mismatch with parent store
valid, conflicts = mvs.ValidateTransactionState(5)
require.False(t, valid)
require.Empty(t, conflicts)
}

func TestMVSValidationWithOnlyEstimate(t *testing.T) {
Expand Down
Loading

0 comments on commit eac8657

Please sign in to comment.