diff --git a/runtime/deferral_test.go b/runtime/deferral_test.go index b554f45f49..6202d9972b 100644 --- a/runtime/deferral_test.go +++ b/runtime/deferral_test.go @@ -27,7 +27,6 @@ import ( "github.com/onflow/cadence" "github.com/onflow/cadence/runtime/common" - "github.com/onflow/cadence/runtime/tests/utils" ) func TestRuntimeStorageDeferredResourceDictionaryValues(t *testing.T) { @@ -146,8 +145,10 @@ func TestRuntimeStorageDeferredResourceDictionaryValues(t *testing.T) { }, } + nextTransactionLocation := newTransactionLocationGenerator() + clearReadsAndWrites() - err := runtime.ExecuteTransaction(deploy, nil, runtimeInterface, utils.TestLocation) + err := runtime.ExecuteTransaction(deploy, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.NotNil(t, accountCode) @@ -155,7 +156,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValues(t *testing.T) { assert.Len(t, writes, 1) clearReadsAndWrites() - err = runtime.ExecuteTransaction(setupTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(setupTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.Len(t, writes, 1) @@ -176,7 +177,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValues(t *testing.T) { `) clearReadsAndWrites() - err = runtime.ExecuteTransaction(insertTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(insertTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) cStorageKey := []byte("storage\x1fc") @@ -215,7 +216,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValues(t *testing.T) { clearReadsAndWrites() loggedMessages = nil - err = runtime.ExecuteTransaction(readTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(readTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.Equal(t, []string{"2", "2"}, loggedMessages) @@ -255,7 +256,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValues(t *testing.T) { clearReadsAndWrites() loggedMessages = nil - err = runtime.ExecuteTransaction(updateTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(updateTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.Equal(t, []string{"3"}, loggedMessages) @@ -308,7 +309,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValues(t *testing.T) { clearReadsAndWrites() loggedMessages = nil - err = runtime.ExecuteTransaction(replaceTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(replaceTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.Equal(t, []string{"3", "4"}, loggedMessages) @@ -361,7 +362,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValues(t *testing.T) { clearReadsAndWrites() loggedMessages = nil - err = runtime.ExecuteTransaction(removeTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(removeTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.Equal(t, []string{"4", "nil"}, loggedMessages) @@ -399,7 +400,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValues(t *testing.T) { clearReadsAndWrites() loggedMessages = nil - err = runtime.ExecuteTransaction(readTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(readTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.Equal(t, []string{"nil", "nil"}, loggedMessages) @@ -440,7 +441,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValues(t *testing.T) { clearReadsAndWrites() loggedMessages = nil - err = runtime.ExecuteTransaction(destroyTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(destroyTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.Equal(t, []string{"1"}, loggedMessages) @@ -613,8 +614,10 @@ func TestRuntimeStorageDeferredResourceDictionaryValuesNested(t *testing.T) { }, } + nextTransactionLocation := newTransactionLocationGenerator() + clearReadsAndWrites() - err := runtime.ExecuteTransaction(deploy, nil, runtimeInterface, utils.TestLocation) + err := runtime.ExecuteTransaction(deploy, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.NotNil(t, accountCode) @@ -622,7 +625,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValuesNested(t *testing.T) { assert.Len(t, writes, 1) clearReadsAndWrites() - err = runtime.ExecuteTransaction(setupTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(setupTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.Len(t, writes, 1) @@ -645,7 +648,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValuesNested(t *testing.T) { `) clearReadsAndWrites() - err = runtime.ExecuteTransaction(insertTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(insertTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) cStorageKey := []byte("storage\x1fc") @@ -687,7 +690,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValuesNested(t *testing.T) { clearReadsAndWrites() loggedMessages = nil - err = runtime.ExecuteTransaction(readTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(readTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.Equal(t, []string{"2"}, loggedMessages) @@ -848,8 +851,10 @@ func TestRuntimeStorageDeferredResourceDictionaryValuesTransfer(t *testing.T) { }, } + nextTransactionLocation := newTransactionLocationGenerator() + clearReadsAndWrites() - err := runtime.ExecuteTransaction(deploy, nil, runtimeInterface, utils.TestLocation) + err := runtime.ExecuteTransaction(deploy, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) assert.NotNil(t, accountCode) @@ -857,7 +862,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValuesTransfer(t *testing.T) { assert.Len(t, writes, 1) clearReadsAndWrites() - err = runtime.ExecuteTransaction(setupTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(setupTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) cStorageKey := []byte("storage\x1fc") @@ -896,7 +901,7 @@ func TestRuntimeStorageDeferredResourceDictionaryValuesTransfer(t *testing.T) { clearReadsAndWrites() loggedMessages = nil - err = runtime.ExecuteTransaction(transferTx, nil, runtimeInterface, utils.TestLocation) + err = runtime.ExecuteTransaction(transferTx, nil, runtimeInterface, nextTransactionLocation()) require.NoError(t, err) require.Len(t, writes, 7) @@ -917,3 +922,151 @@ func TestRuntimeStorageDeferredResourceDictionaryValuesTransfer(t *testing.T) { assert.NotEmpty(t, indexedWrites[string(signer2[:])][string(bStorageKey2)]) assert.NotEmpty(t, indexedWrites[string(signer2[:])][string(xStorageKey2)]) } + +func TestRuntimeStorageDeferredResourceDictionaryValuesRemoval(t *testing.T) { + + // Test that `remove` function correctly loads the potentially deferred value + + runtime := NewInterpreterRuntime() + + contract := []byte(` + pub contract Test { + + pub resource NFT {} + + pub resource Collection { + pub var ownedNFTs: @{UInt64: NFT} + + init () { + self.ownedNFTs <- {UInt64(1): <- create NFT()} + } + + pub fun withdraw(withdrawID: UInt64): @NFT { + let token <- self.ownedNFTs.remove(key: withdrawID) ?? panic("missing NFT") + return <-token + } + + pub fun deposit(token: @NFT) { + let token <- token as! @NFT + let oldToken <- self.ownedNFTs[UInt64(1)] <- token + destroy oldToken + } + + destroy() { + destroy self.ownedNFTs + } + } + + pub fun createCollection(): @Collection { + return <-create Collection() + } + } + `) + + deployTx := []byte(fmt.Sprintf( + ` + transaction { + + prepare(signer: AuthAccount) { + signer.setCode(%s) + } + } + `, + ArrayValueFromBytes(contract).String(), + )) + + setupTx := []byte(` + import Test from 0x1 + + transaction { + + prepare(signer: AuthAccount) { + let collection <- Test.createCollection() + + let token <- collection.withdraw(withdrawID: 1) + + collection.deposit(token: <-token) + + signer.save(<-collection, to: /storage/NFTCollection) + } + } + `) + + testTx := []byte(` + import Test from 0x1 + + transaction { + + prepare(signer: AuthAccount) { + let collection <- signer.load<@Test.Collection>(from:/storage/NFTCollection)! + let nft <- collection.withdraw(withdrawID: 1) + destroy nft + destroy collection + } + } + `) + + var accountCode []byte + var events []cadence.Event + var loggedMessages []string + var reads []testRead + var writes []testWrite + + onRead := func(controller, owner, key, value []byte) { + reads = append(reads, testRead{ + controller, + owner, + key, + }) + } + + onWrite := func(controller, owner, key, value []byte) { + writes = append(writes, testWrite{ + controller, + owner, + key, + value, + }) + } + + clearReadsAndWrites := func() { + writes = nil + reads = nil + } + + signer := common.BytesToAddress([]byte{0x1}) + + runtimeInterface := &testRuntimeInterface{ + resolveImport: func(_ Location) (bytes []byte, err error) { + return accountCode, nil + }, + storage: newTestStorage(onRead, onWrite), + getSigningAccounts: func() []Address { + return []Address{signer} + }, + updateAccountCode: func(address Address, code []byte, checkPermission bool) (err error) { + accountCode = code + return nil + }, + emitEvent: func(event cadence.Event) { + events = append(events, event) + }, + log: func(message string) { + loggedMessages = append(loggedMessages, message) + }, + } + + nextTransactionLocation := newTransactionLocationGenerator() + + clearReadsAndWrites() + err := runtime.ExecuteTransaction(deployTx, nil, runtimeInterface, nextTransactionLocation()) + require.NoError(t, err) + + clearReadsAndWrites() + err = runtime.ExecuteTransaction(setupTx, nil, runtimeInterface, nextTransactionLocation()) + require.NoError(t, err) + + clearReadsAndWrites() + err = runtime.ExecuteTransaction(testTx, nil, runtimeInterface, nextTransactionLocation()) + require.NoError(t, err) +} diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index ddfb001055..a7bcd98cf8 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -5221,7 +5221,7 @@ func dictionaryKey(keyValue Value) string { return hasKeyString.KeyString() } -func (v *DictionaryValue) Set(inter *Interpreter, _ LocationRange, keyValue Value, value Value) { +func (v *DictionaryValue) Set(inter *Interpreter, locationRange LocationRange, keyValue Value, value Value) { v.modified = true switch typedValue := value.(type) { @@ -5229,7 +5229,7 @@ func (v *DictionaryValue) Set(inter *Interpreter, _ LocationRange, keyValue Valu v.Insert(keyValue, typedValue.Value) case NilValue: - v.Remove(inter, keyValue) + v.Remove(inter, locationRange, keyValue) return default: @@ -5282,19 +5282,8 @@ func (v *DictionaryValue) GetMember(_ *Interpreter, _ LocationRange, name string return NewHostFunctionValue( func(invocation Invocation) trampoline.Trampoline { keyValue := invocation.Arguments[0] - - existingValue := v.Remove(invocation.Interpreter, keyValue) - - var returnValue Value - if existingValue == nil { - returnValue = NilValue{} - } else { - returnValue = NewSomeValueOwningNonCopying(existingValue) - } - - return trampoline.Done{ - Result: returnValue, - } + result := v.Remove(invocation.Interpreter, invocation.LocationRange, keyValue) + return trampoline.Done{Result: result} }, ) @@ -5334,11 +5323,13 @@ func (v *DictionaryValue) Count() int { } // TODO: unset owner? -func (v *DictionaryValue) Remove(inter *Interpreter, keyValue Value) (existingValue Value) { +func (v *DictionaryValue) Remove(inter *Interpreter, locationRange LocationRange, keyValue Value) OptionalValue { v.modified = true + // Don't use `Entries` here: the value might be deferred and needs to be loaded + value := v.Get(inter, locationRange, keyValue) + key := dictionaryKey(keyValue) - existingValue, exists := v.Entries[key] // If a resource that was previously deferred is removed from the dictionary, // we delete its old key in storage, and then rely on resource semantics @@ -5350,21 +5341,28 @@ func (v *DictionaryValue) Remove(inter *Interpreter, keyValue Value) (existingVa } } - if !exists { - return nil - } + switch value := value.(type) { + case *SomeValue: - delete(v.Entries, key) + delete(v.Entries, key) - // TODO: optimize linear scan - for i, keyValue := range v.Keys.Values { - if dictionaryKey(keyValue) == key { - v.Keys.Remove(i) - return existingValue + // TODO: optimize linear scan + for i, keyValue := range v.Keys.Values { + if dictionaryKey(keyValue) == key { + v.Keys.Remove(i) + return value + } } - } - panic(errors.NewUnreachableError()) + // Should never occur, the key should have been found + panic(errors.NewUnreachableError()) + + case NilValue: + return value + + default: + panic(errors.NewUnreachableError()) + } } func (v *DictionaryValue) Insert(keyValue Value, value Value) (existingValue Value) { diff --git a/runtime/tests/interpreter/interpreter_test.go b/runtime/tests/interpreter/interpreter_test.go index 47bf91b4de..7181f9c9b1 100644 --- a/runtime/tests/interpreter/interpreter_test.go +++ b/runtime/tests/interpreter/interpreter_test.go @@ -5044,7 +5044,7 @@ func TestInterpretDictionaryRemove(t *testing.T) { interpreter.NewStringValue("abc"), interpreter.NewIntValueFromInt64(1), interpreter.NewStringValue("def"), interpreter.NewIntValueFromInt64(2), ).Copy().(*interpreter.DictionaryValue) - expectedDict.Remove(nil, interpreter.NewStringValue("abc")) + expectedDict.Remove(nil, interpreter.LocationRange{}, interpreter.NewStringValue("abc")) actualDict := inter.Globals["xs"].Value.(*interpreter.DictionaryValue)