TrueCloudLab/neoneo-go
7
0
Fork 2
Code Issues Pull requests Projects Releases Packages Wiki Activity

mpt: allow to remove non-existent keys in batch

Browse source 
This bug was here before batch were intoduced.
`Delete` is allowed to be called on missing keys with
HALT result, MPT needs to take this into account.
This commit is contained in:
Evgeniy Stratonikov 2021-02-17 12:26:32 +03:00
parent 4d0681d898
commit 0cb6ec7345
6 changed files with 37 additions and 15 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

12
pkg/core/blockchain_test.go
View file

@ -1591,3 +1591,15 @@ func TestInvalidNotification(t *testing.T) {
require.Nil(t, aer.Stack[0])
require.Equal(t, stackitem.InteropT, aer.Stack[1].Type())
}
// Test that deletion of non-existent doesn't result in error in tx or block addition.
func TestMPTDeleteNoKey(t *testing.T) {
bc := newTestChain(t)
defer bc.Close()
cs, _ := getTestContractState(bc)
require.NoError(t, bc.contracts.Management.PutContractState(bc.dao, cs))
aer, err := invokeContractMethod(bc, 1_00000000, cs.Hash, "delValue", "non-existent-key")
require.NoError(t, err)
require.Equal(t, vm.HaltState, aer.VMState)
}

12
pkg/core/interop_system_test.go
View file

@ -342,6 +342,10 @@ func getTestContractState(bc *Blockchain) (*state.Contract, *state.Contract) {
emit.Syscall(w.BinWriter, interopnames.SystemStorageGetContext)
emit.Syscall(w.BinWriter, interopnames.SystemStorageGet)
emit.Opcodes(w.BinWriter, opcode.RET)
delValOff := w.Len()
emit.Syscall(w.BinWriter, interopnames.SystemStorageGetContext)
emit.Syscall(w.BinWriter, interopnames.SystemStorageDelete)
emit.Opcodes(w.BinWriter, opcode.RET)
onNEP17PaymentOff := w.Len()
emit.Syscall(w.BinWriter, interopnames.SystemRuntimeGetCallingScriptHash)
emit.Int(w.BinWriter, 4)
@ -464,6 +468,14 @@ func getTestContractState(bc *Blockchain) (*state.Contract, *state.Contract) {
},
ReturnType: smartcontract.VoidType,
},
{
Name: "delValue",
Offset: delValOff,
Parameters: []manifest.Parameter{
manifest.NewParameter("key", smartcontract.StringType),
},
ReturnType: smartcontract.VoidType,
},
{
Name: manifest.MethodOnNEP11Payment,
Offset: onNEP11PaymentOff,

6
pkg/core/mpt/batch.go
View file

@ -207,10 +207,8 @@ func (t *Trie) newSubTrieMany(prefix []byte, kv []keyValue, value []byte) (Node,
if len(kv[0].key) == 0 {
if len(kv[0].value) == 0 {
if len(kv) == 1 {
if len(value) != 0 {
return new(HashNode), 1, nil
}
return new(HashNode), 0, ErrNotFound
return new(HashNode), 1, nil
}
node, n, err := t.newSubTrieMany(prefix, kv[1:], nil)
return node, n + 1, err

6
pkg/core/mpt/batch_test.go
View file

@ -170,7 +170,7 @@ func TestTrie_PutBatchBranch(t *testing.T) {
{[]byte{0x20, 2}, nil},
{[]byte{0x30, 3}, []byte("won't be put")},
}
testIncompletePut(t, ps, 1, tr1, tr2)
testIncompletePut(t, ps, 3, tr1, tr2)
})
t.Run("incomplete put, transform to empty", func(t *testing.T) {
tr1, tr2 := prepareBranch(t)
@ -180,7 +180,7 @@ func TestTrie_PutBatchBranch(t *testing.T) {
{[]byte{0x20, 2}, nil},
{[]byte{0x30, 3}, []byte("won't be put")},
}
testIncompletePut(t, ps, 2, tr1, tr2)
testIncompletePut(t, ps, 4, tr1, tr2)
})
t.Run("remove 2, become empty", func(t *testing.T) {
tr1, tr2 := prepareBranch(t)
@ -247,7 +247,7 @@ func TestTrie_PutBatchEmpty(t *testing.T) {
}
tr1 := NewTrie(new(HashNode), false, newTestStore())
tr2 := NewTrie(new(HashNode), false, newTestStore())
testIncompletePut(t, ps, 2, tr1, tr2)
testIncompletePut(t, ps, 4, tr1, tr2)
})
}

6
pkg/core/mpt/trie.go
View file

@ -290,7 +290,7 @@ func (t *Trie) deleteFromBranch(b *BranchNode, path []byte) (Node, error) {
func (t *Trie) deleteFromExtension(n *ExtensionNode, path []byte) (Node, error) {
if !bytes.HasPrefix(path, n.key) {
return nil, ErrNotFound
return n, nil
}
h := n.Hash()
bs := n.bytes
@ -325,14 +325,14 @@ func (t *Trie) deleteFromNode(curr Node, path []byte) (Node, error) {
t.removeRef(curr.Hash(), curr.Bytes())
return new(HashNode), nil
}
return nil, ErrNotFound
return curr, nil
case *BranchNode:
return t.deleteFromBranch(n, path)
case *ExtensionNode:
return t.deleteFromExtension(n, path)
case *HashNode:
if n.IsEmpty() {
return nil, ErrNotFound
return n, nil
}
newNode, err := t.getFromStore(n.Hash())
if err != nil {

10
pkg/core/mpt/trie_test.go
View file

@ -62,8 +62,8 @@ func testTrieRefcount(t *testing.T, key1, key2 []byte) {
tr.testHas(t, key1, nil)
tr.testHas(t, key2, []byte{1})
// error on delete, refcount should not be updated
require.Error(t, tr.Delete(key1))
// delete non-existent, refcount should not be updated
require.NoError(t, tr.Delete(key1))
tr.Flush()
tr.testHas(t, key1, nil)
tr.testHas(t, key2, []byte{1})
@ -316,7 +316,7 @@ func testTrieDelete(t *testing.T, enableGC bool) {
t.Run("Empty", func(t *testing.T) {
tr := NewTrie(nil, enableGC, newTestStore())
require.Error(t, tr.Delete([]byte{}))
require.NoError(t, tr.Delete([]byte{}))
})
})
@ -324,7 +324,7 @@ func testTrieDelete(t *testing.T, enableGC bool) {
l := NewLeafNode([]byte{0x12, 0x34})
tr := NewTrie(l, enableGC, newTestStore())
t.Run("NonExistentKey", func(t *testing.T) {
require.Error(t, tr.Delete([]byte{0x12}))
require.NoError(t, tr.Delete([]byte{0x12}))
tr.testHas(t, []byte{}, []byte{0x12, 0x34})
})
require.NoError(t, tr.Delete([]byte{}))
@ -338,7 +338,7 @@ func testTrieDelete(t *testing.T, enableGC bool) {
tr := NewTrie(e, enableGC, newTestStore())
t.Run("NonExistentKey", func(t *testing.T) {
require.Error(t, tr.Delete([]byte{}))
require.NoError(t, tr.Delete([]byte{}))
tr.testHas(t, []byte{0xAB}, []byte{0x12, 0x34})
})