From a36e4ec6766ee4bfafcfe110b14688d78192bd7d Mon Sep 17 00:00:00 2001 From: jimboj Date: Fri, 23 Aug 2024 16:12:50 +0800 Subject: [PATCH 1/3] add child prefix check --- lib/runtime/storage/trie.go | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/lib/runtime/storage/trie.go b/lib/runtime/storage/trie.go index c8a1eb5828..4dff21dead 100644 --- a/lib/runtime/storage/trie.go +++ b/lib/runtime/storage/trie.go @@ -18,6 +18,8 @@ import ( "golang.org/x/exp/slices" ) +var childStorageKeyPrefix = []byte(":child_storage:") + // TrieState relies on `storageDiff` to perform changes over the current state. // It has support for transactions using "nested" storageDiff changes // If the execution of the call is successful, the changes will be applied to @@ -206,11 +208,20 @@ func (t *TrieState) NextKey(key []byte) []byte { return t.state.NextKey(key) } +// Check if the given prefix starts with the child storage key prefix +func startsWithChildStorageKey(prefix []byte) bool { + return bytes.HasPrefix(prefix, childStorageKeyPrefix) +} + // ClearPrefix deletes all key-value pairs from the trie where the key starts with the given prefix func (t *TrieState) ClearPrefix(prefix []byte) error { t.mtx.Lock() defer t.mtx.Unlock() + if startsWithChildStorageKey(prefix) { + return fmt.Errorf("cannot clear prefix that is part of or contains a child storage key") + } + if currentTx := t.getCurrentTransaction(); currentTx != nil { keysOnState := make([]string, 0) @@ -232,6 +243,10 @@ func (t *TrieState) ClearPrefixLimit(prefix []byte, limit uint32) ( t.mtx.Lock() defer t.mtx.Unlock() + if startsWithChildStorageKey(prefix) { + return deleted, allDeleted, fmt.Errorf("cannot clear prefix that is part of or contains a child storage key") + } + if currentTx := t.getCurrentTransaction(); currentTx != nil { keysOnState := make([]string, 0) From 126621c0cb2c7738090224813f1d0efe8a53a15f Mon Sep 17 00:00:00 2001 From: jimboj Date: Fri, 23 Aug 2024 16:18:20 +0800 Subject: [PATCH 2/3] move logic into imports for better logging --- lib/runtime/storage/trie.go | 15 ----------- lib/runtime/wazero/imports.go | 50 ++++++++++++++++++++++------------- 2 files changed, 31 insertions(+), 34 deletions(-) diff --git a/lib/runtime/storage/trie.go b/lib/runtime/storage/trie.go index 4dff21dead..c8a1eb5828 100644 --- a/lib/runtime/storage/trie.go +++ b/lib/runtime/storage/trie.go @@ -18,8 +18,6 @@ import ( "golang.org/x/exp/slices" ) -var childStorageKeyPrefix = []byte(":child_storage:") - // TrieState relies on `storageDiff` to perform changes over the current state. // It has support for transactions using "nested" storageDiff changes // If the execution of the call is successful, the changes will be applied to @@ -208,20 +206,11 @@ func (t *TrieState) NextKey(key []byte) []byte { return t.state.NextKey(key) } -// Check if the given prefix starts with the child storage key prefix -func startsWithChildStorageKey(prefix []byte) bool { - return bytes.HasPrefix(prefix, childStorageKeyPrefix) -} - // ClearPrefix deletes all key-value pairs from the trie where the key starts with the given prefix func (t *TrieState) ClearPrefix(prefix []byte) error { t.mtx.Lock() defer t.mtx.Unlock() - if startsWithChildStorageKey(prefix) { - return fmt.Errorf("cannot clear prefix that is part of or contains a child storage key") - } - if currentTx := t.getCurrentTransaction(); currentTx != nil { keysOnState := make([]string, 0) @@ -243,10 +232,6 @@ func (t *TrieState) ClearPrefixLimit(prefix []byte, limit uint32) ( t.mtx.Lock() defer t.mtx.Unlock() - if startsWithChildStorageKey(prefix) { - return deleted, allDeleted, fmt.Errorf("cannot clear prefix that is part of or contains a child storage key") - } - if currentTx := t.getCurrentTransaction(); currentTx != nil { keysOnState := make([]string, 0) diff --git a/lib/runtime/wazero/imports.go b/lib/runtime/wazero/imports.go index 71ef19928a..32535f5774 100644 --- a/lib/runtime/wazero/imports.go +++ b/lib/runtime/wazero/imports.go @@ -37,6 +37,8 @@ var ( emptyByteVectorEncoded []byte = scale.MustMarshal([]byte{}) noneEncoded []byte = []byte{0x00} allZeroesBytes = [32]byte{} + + childStorageKeyPrefix = []byte(":child_storage:") ) const ( @@ -2065,10 +2067,15 @@ func ext_storage_clear_prefix_version_1(ctx context.Context, m api.Module, prefi prefix := read(m, prefixSpan) logger.Debugf("prefix: 0x%x", prefix) - err := storage.ClearPrefix(prefix) - if err != nil { - panic(err) + if !bytes.HasPrefix(prefix, childStorageKeyPrefix) { + err := storage.ClearPrefix(prefix) + if err != nil { + panic(err) + } + } else { + logger.Warnf("cannot clear prefix that is part of or contains a child storage key") } + } // toKillStorageResultEnum encodes the `allRemoved` flag and @@ -2117,25 +2124,30 @@ func ext_storage_clear_prefix_version_2(ctx context.Context, m api.Module, prefi limitPtr = &maxLimit } - numRemoved, all, err := storage.ClearPrefixLimit(prefix, *limitPtr) - if err != nil { - logger.Errorf("failed to clear prefix limit: %s", err) - panic(err) - } + if !bytes.HasPrefix(prefix, childStorageKeyPrefix) { + numRemoved, all, err := storage.ClearPrefixLimit(prefix, *limitPtr) + if err != nil { + logger.Errorf("failed to clear prefix limit: %s", err) + panic(err) + } - encBytes, err := toKillStorageResultEnum(all, numRemoved) - if err != nil { - logger.Errorf("failed to allocate memory: %s", err) - panic(err) - } + encBytes, err := toKillStorageResultEnum(all, numRemoved) + if err != nil { + logger.Errorf("failed to allocate memory: %s", err) + panic(err) + } - valueSpan, err := write(m, rtCtx.Allocator, encBytes) - if err != nil { - logger.Errorf("failed to allocate: %s", err) - panic(err) - } + valueSpan, err := write(m, rtCtx.Allocator, encBytes) + if err != nil { + logger.Errorf("failed to allocate: %s", err) + panic(err) + } - return valueSpan + return valueSpan + } else { + logger.Warnf("cannot clear prefix that is part of or contains a child storage key") + } + return 0 } func ext_storage_exists_version_1(ctx context.Context, m api.Module, keySpan uint64) uint32 { From d4e7107392b13aec638ff06f9960a4d402b7eb20 Mon Sep 17 00:00:00 2001 From: jimboj Date: Mon, 26 Aug 2024 15:04:19 +0800 Subject: [PATCH 3/3] respond to feedback --- lib/runtime/wazero/imports.go | 39 ++++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/lib/runtime/wazero/imports.go b/lib/runtime/wazero/imports.go index 32535f5774..b077d19731 100644 --- a/lib/runtime/wazero/imports.go +++ b/lib/runtime/wazero/imports.go @@ -2124,30 +2124,35 @@ func ext_storage_clear_prefix_version_2(ctx context.Context, m api.Module, prefi limitPtr = &maxLimit } - if !bytes.HasPrefix(prefix, childStorageKeyPrefix) { - numRemoved, all, err := storage.ClearPrefixLimit(prefix, *limitPtr) + if bytes.HasPrefix(prefix, childStorageKeyPrefix) { + logger.Warnf("cannot clear child prefix: 0x%x", prefix) + encBytes, err := toKillStorageResultEnum(false, 0) if err != nil { - logger.Errorf("failed to clear prefix limit: %s", err) panic(err) } - encBytes, err := toKillStorageResultEnum(all, numRemoved) - if err != nil { - logger.Errorf("failed to allocate memory: %s", err) - panic(err) - } + valueSpan := mustWrite(m, rtCtx.Allocator, encBytes) + return valueSpan + } - valueSpan, err := write(m, rtCtx.Allocator, encBytes) - if err != nil { - logger.Errorf("failed to allocate: %s", err) - panic(err) - } + numRemoved, all, err := storage.ClearPrefixLimit(prefix, *limitPtr) + if err != nil { + logger.Errorf("failed to clear prefix limit: %s", err) + panic(err) + } - return valueSpan - } else { - logger.Warnf("cannot clear prefix that is part of or contains a child storage key") + encBytes, err := toKillStorageResultEnum(all, numRemoved) + if err != nil { + logger.Errorf("failed to allocate memory: %s", err) + panic(err) } - return 0 + + valueSpan, err := write(m, rtCtx.Allocator, encBytes) + if err != nil { + logger.Errorf("failed to allocate: %s", err) + panic(err) + } + return valueSpan } func ext_storage_exists_version_1(ctx context.Context, m api.Module, keySpan uint64) uint32 {