Skip to content

Commit 2dde9fd

Browse files
Merge #4195
4195: Misc state/view usage cleanup r=pattyshack a=pattyshack Co-authored-by: Patrick Lee <[email protected]>
2 parents f0c0e20 + bb153e0 commit 2dde9fd

File tree

7 files changed

+77
-75
lines changed

7 files changed

+77
-75
lines changed

engine/execution/computation/manager_test.go

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ import (
2525
"github.com/onflow/flow-go/engine/execution/computation/computer"
2626
"github.com/onflow/flow-go/engine/execution/computation/query"
2727
state2 "github.com/onflow/flow-go/engine/execution/state"
28-
"github.com/onflow/flow-go/engine/execution/state/delta"
2928
unittest2 "github.com/onflow/flow-go/engine/execution/state/unittest"
3029
"github.com/onflow/flow-go/engine/execution/testutil"
3130
"github.com/onflow/flow-go/fvm"
@@ -211,17 +210,13 @@ func TestComputeBlock_Uploader(t *testing.T) {
211210
derivedChainData: derivedChainData,
212211
}
213212

214-
view := delta.NewDeltaView(
215-
state2.NewLedgerStorageSnapshot(
216-
ledger,
217-
flow.StateCommitment(ledger.InitialState())))
218-
blockView := view.NewChild()
219-
220213
_, err = manager.ComputeBlock(
221214
context.Background(),
222215
unittest.IdentifierFixture(),
223216
computationResult.ExecutableBlock,
224-
blockView)
217+
state2.NewLedgerStorageSnapshot(
218+
ledger,
219+
flow.StateCommitment(ledger.InitialState())))
225220
require.NoError(t, err)
226221
}
227222

@@ -913,7 +908,8 @@ func TestScriptStorageMutationsDiscarded(t *testing.T) {
913908
cadence.NewPath("storage", "x"),
914909
)
915910

916-
// the save should not update account storage by writing the delta from the child view back to the parent
911+
// the save should not update account storage by writing the updates
912+
// back to the snapshotTree
917913
require.NoError(t, err)
918914
require.Equal(t, nil, v)
919915
}

engine/execution/state/state_test.go

Lines changed: 52 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import (
1414
"github.com/onflow/flow-go/ledger/common/pathfinder"
1515

1616
"github.com/onflow/flow-go/engine/execution/state"
17-
"github.com/onflow/flow-go/engine/execution/state/delta"
17+
fvmstate "github.com/onflow/flow-go/fvm/state"
1818
ledger "github.com/onflow/flow-go/ledger/complete"
1919
"github.com/onflow/flow-go/ledger/complete/wal/fixtures"
2020
"github.com/onflow/flow-go/model/flow"
@@ -77,14 +77,14 @@ func TestExecutionStateWithTrieStorage(t *testing.T) {
7777
sc1, err := es.StateCommitmentByBlockID(context.Background(), flow.Identifier{})
7878
assert.NoError(t, err)
7979

80-
view1 := delta.NewDeltaView(es.NewStorageSnapshot(sc1))
80+
executionSnapshot := &fvmstate.ExecutionSnapshot{
81+
WriteSet: map[flow.RegisterID]flow.RegisterValue{
82+
registerID1: flow.RegisterValue("apple"),
83+
registerID2: flow.RegisterValue("carrot"),
84+
},
85+
}
8186

82-
err = view1.Set(registerID1, flow.RegisterValue("apple"))
83-
assert.NoError(t, err)
84-
err = view1.Set(registerID2, flow.RegisterValue("carrot"))
85-
assert.NoError(t, err)
86-
87-
sc2, update, err := state.CommitDelta(l, view1.Finalize(), sc1)
87+
sc2, update, err := state.CommitDelta(l, executionSnapshot, sc1)
8888
assert.NoError(t, err)
8989

9090
assert.Equal(t, sc1[:], update.RootHash[:])
@@ -122,11 +122,11 @@ func TestExecutionStateWithTrieStorage(t *testing.T) {
122122
assert.Equal(t, []byte("apple"), []byte(update.Payloads[0].Value()))
123123
assert.Equal(t, []byte("carrot"), []byte(update.Payloads[1].Value()))
124124

125-
view2 := delta.NewDeltaView(es.NewStorageSnapshot(sc2))
125+
storageSnapshot := es.NewStorageSnapshot(sc2)
126126

127-
b1, err := view2.Get(registerID1)
127+
b1, err := storageSnapshot.Get(registerID1)
128128
assert.NoError(t, err)
129-
b2, err := view2.Get(registerID2)
129+
b2, err := storageSnapshot.Get(registerID2)
130130
assert.NoError(t, err)
131131

132132
assert.Equal(t, flow.RegisterValue("apple"), b1)
@@ -138,32 +138,36 @@ func TestExecutionStateWithTrieStorage(t *testing.T) {
138138
sc1, err := es.StateCommitmentByBlockID(context.Background(), flow.Identifier{})
139139
assert.NoError(t, err)
140140

141-
view1 := delta.NewDeltaView(es.NewStorageSnapshot(sc1))
141+
executionSnapshot1 := &fvmstate.ExecutionSnapshot{
142+
WriteSet: map[flow.RegisterID]flow.RegisterValue{
143+
registerID1: []byte("apple"),
144+
},
145+
}
142146

143-
err = view1.Set(registerID1, []byte("apple"))
144-
assert.NoError(t, err)
145-
sc2, _, err := state.CommitDelta(l, view1.Finalize(), sc1)
147+
sc2, _, err := state.CommitDelta(l, executionSnapshot1, sc1)
146148
assert.NoError(t, err)
147149

148150
// update value and get resulting state commitment
149-
view2 := delta.NewDeltaView(es.NewStorageSnapshot(sc2))
150-
err = view2.Set(registerID1, []byte("orange"))
151-
assert.NoError(t, err)
151+
executionSnapshot2 := &fvmstate.ExecutionSnapshot{
152+
WriteSet: map[flow.RegisterID]flow.RegisterValue{
153+
registerID1: []byte("orange"),
154+
},
155+
}
152156

153-
sc3, _, err := state.CommitDelta(l, view2.Finalize(), sc2)
157+
sc3, _, err := state.CommitDelta(l, executionSnapshot2, sc2)
154158
assert.NoError(t, err)
155159

156160
// create a view for previous state version
157-
view3 := delta.NewDeltaView(es.NewStorageSnapshot(sc2))
161+
storageSnapshot3 := es.NewStorageSnapshot(sc2)
158162

159163
// create a view for new state version
160-
view4 := delta.NewDeltaView(es.NewStorageSnapshot(sc3))
164+
storageSnapshot4 := es.NewStorageSnapshot(sc3)
161165

162166
// fetch the value at both versions
163-
b1, err := view3.Get(registerID1)
167+
b1, err := storageSnapshot3.Get(registerID1)
164168
assert.NoError(t, err)
165169

166-
b2, err := view4.Get(registerID1)
170+
b2, err := storageSnapshot4.Get(registerID1)
167171
assert.NoError(t, err)
168172

169173
assert.Equal(t, flow.RegisterValue("apple"), b1)
@@ -176,34 +180,37 @@ func TestExecutionStateWithTrieStorage(t *testing.T) {
176180
assert.NoError(t, err)
177181

178182
// set initial value
179-
view1 := delta.NewDeltaView(es.NewStorageSnapshot(sc1))
180-
err = view1.Set(registerID1, []byte("apple"))
181-
assert.NoError(t, err)
182-
err = view1.Set(registerID2, []byte("apple"))
183-
assert.NoError(t, err)
183+
executionSnapshot1 := &fvmstate.ExecutionSnapshot{
184+
WriteSet: map[flow.RegisterID]flow.RegisterValue{
185+
registerID1: []byte("apple"),
186+
registerID2: []byte("apple"),
187+
},
188+
}
184189

185-
sc2, _, err := state.CommitDelta(l, view1.Finalize(), sc1)
190+
sc2, _, err := state.CommitDelta(l, executionSnapshot1, sc1)
186191
assert.NoError(t, err)
187192

188193
// update value and get resulting state commitment
189-
view2 := delta.NewDeltaView(es.NewStorageSnapshot(sc2))
190-
err = view2.Set(registerID1, nil)
191-
assert.NoError(t, err)
194+
executionSnapshot2 := &fvmstate.ExecutionSnapshot{
195+
WriteSet: map[flow.RegisterID]flow.RegisterValue{
196+
registerID1: nil,
197+
},
198+
}
192199

193-
sc3, _, err := state.CommitDelta(l, view2.Finalize(), sc2)
200+
sc3, _, err := state.CommitDelta(l, executionSnapshot2, sc2)
194201
assert.NoError(t, err)
195202

196203
// create a view for previous state version
197-
view3 := delta.NewDeltaView(es.NewStorageSnapshot(sc2))
204+
storageSnapshot3 := es.NewStorageSnapshot(sc2)
198205

199206
// create a view for new state version
200-
view4 := delta.NewDeltaView(es.NewStorageSnapshot(sc3))
207+
storageSnapshot4 := es.NewStorageSnapshot(sc3)
201208

202209
// fetch the value at both versions
203-
b1, err := view3.Get(registerID1)
210+
b1, err := storageSnapshot3.Get(registerID1)
204211
assert.NoError(t, err)
205212

206-
b2, err := view4.Get(registerID1)
213+
b2, err := storageSnapshot4.Get(registerID1)
207214
assert.NoError(t, err)
208215

209216
assert.Equal(t, flow.RegisterValue("apple"), b1)
@@ -216,17 +223,18 @@ func TestExecutionStateWithTrieStorage(t *testing.T) {
216223
assert.NoError(t, err)
217224

218225
// set initial value
219-
view1 := delta.NewDeltaView(es.NewStorageSnapshot(sc1))
220-
err = view1.Set(registerID1, flow.RegisterValue("apple"))
221-
assert.NoError(t, err)
222-
err = view1.Set(registerID2, flow.RegisterValue("apple"))
223-
assert.NoError(t, err)
226+
executionSnapshot1 := &fvmstate.ExecutionSnapshot{
227+
WriteSet: map[flow.RegisterID]flow.RegisterValue{
228+
registerID1: flow.RegisterValue("apple"),
229+
registerID2: flow.RegisterValue("apple"),
230+
},
231+
}
224232

225-
sc2, _, err := state.CommitDelta(l, view1.Finalize(), sc1)
233+
sc2, _, err := state.CommitDelta(l, executionSnapshot1, sc1)
226234
assert.NoError(t, err)
227235

228236
// committing for the second time should be OK
229-
sc2Same, _, err := state.CommitDelta(l, view1.Finalize(), sc1)
237+
sc2Same, _, err := state.CommitDelta(l, executionSnapshot1, sc1)
230238
assert.NoError(t, err)
231239

232240
require.Equal(t, sc2, sc2Same)

fvm/derived/table_test.go

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1089,7 +1089,10 @@ func TestDerivedDataTableGetOrCompute(t *testing.T) {
10891089
assert.Equal(t, value, val)
10901090
assert.True(t, computer.called)
10911091

1092-
_, found := view.Finalize().ReadSet[key]
1092+
snapshot, err := txnState.FinalizeMainTransaction()
1093+
assert.NoError(t, err)
1094+
1095+
_, found := snapshot.ReadSet[key]
10931096
assert.True(t, found)
10941097

10951098
// Commit to setup the next test.
@@ -1112,7 +1115,10 @@ func TestDerivedDataTableGetOrCompute(t *testing.T) {
11121115
assert.Equal(t, value, val)
11131116
assert.False(t, computer.called)
11141117

1115-
_, found := view.Finalize().ReadSet[key]
1118+
snapshot, err := txnState.FinalizeMainTransaction()
1119+
assert.NoError(t, err)
1120+
1121+
_, found := snapshot.ReadSet[key]
11161122
assert.True(t, found)
11171123
})
11181124
}

fvm/environment/accounts_test.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,11 @@ func TestAccounts_Create(t *testing.T) {
2323
err := accounts.Create(nil, address)
2424
require.NoError(t, err)
2525

26+
snapshot, err := txnState.FinalizeMainTransaction()
27+
require.NoError(t, err)
28+
2629
// account status
27-
require.Equal(t, len(txnState.Finalize().AllRegisterIDs()), 1)
30+
require.Equal(t, len(snapshot.AllRegisterIDs()), 1)
2831
})
2932

3033
t.Run("Fails if account exists", func(t *testing.T) {

fvm/environment/derived_data_invalidator_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -301,7 +301,7 @@ func TestMeterParamOverridesUpdated(t *testing.T) {
301301
require.Equal(t, expected, invalidator.MeterParamOverridesUpdated)
302302
}
303303

304-
executionSnapshot, err = nestedTxn.FinalizeMainTransaction()
304+
executionSnapshot, err = txnState.FinalizeMainTransaction()
305305
require.NoError(t, err)
306306

307307
for _, registerId := range executionSnapshot.AllRegisterIDs() {

fvm/state/execution_state_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -130,7 +130,7 @@ func TestExecutionState_ChildMergeFunctionality(t *testing.T) {
130130
require.NoError(t, err)
131131

132132
// now should be part of the ledger
133-
v, err := view.Get(key)
133+
v, err := st.Get(key)
134134
require.NoError(t, err)
135135
require.Equal(t, v, value)
136136
})

fvm/storage/testutils/utils.go

Lines changed: 6 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -7,32 +7,21 @@ import (
77
"github.com/onflow/flow-go/fvm/storage"
88
)
99

10-
type SimpleTestTransaction struct {
11-
*delta.View
12-
13-
storage.SerialTransaction
14-
}
15-
1610
// NewSimpleTransaction returns a transaction which can be used to test
1711
// fvm evaluation. The returned transaction should not be committed.
1812
func NewSimpleTransaction(
1913
snapshot state.StorageSnapshot,
20-
) *SimpleTestTransaction {
21-
view := delta.NewDeltaView(snapshot)
22-
14+
) *storage.SerialTransaction {
2315
derivedBlockData := derived.NewEmptyDerivedBlockData()
2416
derivedTxnData, err := derivedBlockData.NewDerivedTransactionData(0, 0)
2517
if err != nil {
2618
panic(err)
2719
}
2820

29-
return &SimpleTestTransaction{
30-
View: view,
31-
SerialTransaction: storage.SerialTransaction{
32-
NestedTransaction: state.NewTransactionState(
33-
view,
34-
state.DefaultParameters()),
35-
DerivedTransactionCommitter: derivedTxnData,
36-
},
21+
return &storage.SerialTransaction{
22+
NestedTransaction: state.NewTransactionState(
23+
delta.NewDeltaView(snapshot),
24+
state.DefaultParameters()),
25+
DerivedTransactionCommitter: derivedTxnData,
3726
}
3827
}

0 commit comments

Comments
 (0)