Skip to content

Commit 33b3c6d

Browse files
asdacapasdacap
committed
Fix self destruct missing clear
1 parent 0797b7b commit 33b3c6d

File tree

6 files changed

+40
-49
lines changed

6 files changed

+40
-49
lines changed

‎src/Nethermind/Nethermind.Db.Rocks/Config/DbConfig.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -508,5 +508,5 @@ public class DbConfig : IDbConfig
508508
TrieNodeConfig +
509509
"";
510510
public string? FlatStorageTopNodesNodesDbAdditionalRocksDbOptions { get; set; }
511-
public ulong PreimageDbWriteBufferSize { get; set; } = (ulong)256.MiB();
511+
public ulong PreimageDbWriteBufferSize { get; set; } = (ulong)16.MiB();
512512
}

‎src/Nethermind/Nethermind.Evm/TransactionProcessing/TransactionProcessor.cs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -636,6 +636,7 @@ private TransactionResult BuildExecutionEnvironment(
636636

637637
protected virtual bool ShouldValidate(ExecutionOptions opts) => !opts.HasFlag(ExecutionOptions.SkipValidation);
638638

639+
[MethodImpl(MethodImplOptions.NoOptimization)]
639640
private int ExecuteEvmCall<TTracingInst>(
640641
Transaction tx,
641642
BlockHeader header,

‎src/Nethermind/Nethermind.State/Flat/FlatDiffRepository.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -691,7 +691,7 @@ public void Add(Snapshot snapshot)
691691
}
692692
if (deleted > 0)
693693
{
694-
_logger.Warn($"Should selfdestruct {toSelfDestructStorage.Key}. Deleted {deleted}");
694+
_logger.Warn($"Should selfdestruct {toSelfDestructStorage.Key}. Deleted {deleted}. Snapshot range {snapshot.From} {snapshot.To}");
695695
throw new Exception($"Should sefl destruct not called properly {toSelfDestructStorage.Key}");
696696
}
697697
continue;

‎src/Nethermind/Nethermind.State/Flat/SnapshotBundle.cs‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,7 @@ public void SetAccount(AddressAsKey addr, Account? account)
464464
{
465465
if (addr == FlatWorldStateScope.DebugAddress)
466466
{
467-
Console.Error.WriteLine("set to {a}");
467+
Console.Error.WriteLine($"set to {account}");
468468
}
469469
_changedAccounts[addr] = account;
470470
}
@@ -549,13 +549,17 @@ public Snapshot CompactToKnownState()
549549
{
550550
var address = addrK.Key;
551551
var isNewAccount = addrK.Value;
552-
selfDestructedStorageAddresses[address] = isNewAccount;
553-
554552
if (!isNewAccount)
555553
{
554+
selfDestructedStorageAddresses[address] = false;
556555
addressToClear.Add(address);
557556
addressHashToClear.Add(address.Value.ToAccountPath.ToCommitment());
558557
}
558+
else
559+
{
560+
// Note, if its already false, we should not set it to true
561+
selfDestructedStorageAddresses.TryAdd(address, true);
562+
}
559563
}
560564

561565
if (addressToClear.Count > 0)
@@ -638,7 +642,7 @@ public void Clear(Address address, Hash256AsKey addressHash)
638642
isNewAccount = account == null;
639643
if (address == FlatWorldStateScope.DebugAddress)
640644
{
641-
Console.Error.WriteLine("The clear is is new");
645+
Console.Error.WriteLine($"The clear newness is {isNewAccount}");
642646
}
643647
}
644648
else

‎src/Nethermind/Nethermind.State/PersistentStorageProvider.cs‎

Lines changed: 15 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
using Nethermind.Evm.Tracing.State;
2222
using Nethermind.Logging;
2323
using Nethermind.Int256;
24+
using Nethermind.State.Flat.ScopeProvider;
2425
using Nethermind.Trie;
2526

2627
namespace Nethermind.State;
@@ -217,7 +218,9 @@ protected override void CommitCore(IStorageTracer tracer)
217218
foreach (AddressAsKey address in toUpdateRoots)
218219
{
219220
// since the accounts could be empty accounts that are removing (EIP-158)
220-
if (_stateProvider.AccountExists(address))
221+
// Note: it could be that the TX remove the account but the underlying account was originally not missing
222+
// hence, the storage Clear need to be called, hence the additional call to _currentScope.Get
223+
if (_stateProvider.AccountExists(address) || _currentScope.Get(address) is not null)
221224
{
222225
_toUpdateRoots[address] = true;
223226
// Add storage tree, will accessed later, which may be in parallel
@@ -271,6 +274,7 @@ void UpdateRootHashesSingleThread()
271274
{
272275
if (!_toUpdateRoots.TryGetValue(kvp.Key, out bool hasChanges) || !hasChanges)
273276
{
277+
if (kvp.Key == FlatWorldStateScope.DebugAddress) Console.Error.WriteLine("Update skipped");
274278
// Wasn't updated don't recalculate
275279
continue;
276280
}
@@ -405,6 +409,11 @@ public override void ClearStorage(Address address)
405409
{
406410
base.ClearStorage(address);
407411

412+
if (address == FlatWorldStateScope.DebugAddress)
413+
{
414+
Console.Error.WriteLine($"Clear on world state for debug address {address}");
415+
}
416+
408417
_toUpdateRoots.TryAdd(address, true);
409418

410419
PerContractState state = GetOrCreateStorage(address);
@@ -418,38 +427,6 @@ private sealed class DefaultableDictionary()
418427
public int EstimatedSize => _dictionary.Count + (_missingAreDefault ? 1 : 0);
419428
public bool HasClear => _missingAreDefault;
420429
public int Capacity => _dictionary.Capacity;
421-
/*
422-
*
423-
<<<<<<< HEAD
424-
=======
425-
// Note: These all run in about 0.4ms. So the little overhead like attempting to sort the tasks
426-
// may make it worse. Always check on mainnet.
427-
428-
using ArrayPoolListRef<Task> commitTask = new(_storages.Count);
429-
foreach (PerContractState storage in _storages.Values)
430-
{
431-
storage.EnsureStorageTree(); // Cannot be called concurrently
432-
if (blockCommitter.TryRequestConcurrencyQuota())
433-
{
434-
commitTask.Add(Task.Factory.StartNew((ctx) =>
435-
{
436-
PerContractState st = (PerContractState)ctx;
437-
st.Commit();
438-
st.Return();
439-
blockCommitter.ReturnConcurrencyQuota();
440-
}, storage));
441-
}
442-
else
443-
{
444-
storage.Commit();
445-
storage.Return();
446-
}
447-
}
448-
449-
Task.WaitAll(commitTask.AsSpan());
450-
451-
>>>>>>> origin/master
452-
*/
453430

454431
public void Reset()
455432
{
@@ -623,6 +600,11 @@ private static byte[] LoadFromTreeStorage(StorageCell storageCell, PerContractSt
623600
int writes = 0;
624601
int skipped = 0;
625602

603+
if (_address == FlatWorldStateScope.DebugAddress && BlockChange.HasClear)
604+
{
605+
Console.Error.WriteLine("Clear in process storage change");
606+
}
607+
626608
if (BlockChange.HasClear)
627609
{
628610
storageWriteBatch.Clear();

‎src/Nethermind/Nethermind.State/StateProvider.cs‎

Lines changed: 14 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
using Nethermind.Evm.Tracing.State;
2323
using Nethermind.Int256;
2424
using Nethermind.Logging;
25+
using Nethermind.State.Flat.ScopeProvider;
2526
using Metrics = Nethermind.Db.Metrics;
2627
using static Nethermind.State.StateProvider;
2728

@@ -177,14 +178,11 @@ static Account ThrowIfNull(Address address)
177178
=> throw new InvalidOperationException($"Account {address} is null when updating code hash");
178179
}
179180

180-
// 0x9290084cde3cfc1085e5e8aa6ff1fb141b3cebc4ac64f4ce937a2c9b5cd85a69, // Hash
181-
private Address importantAddress = new Address("0xf664829682daf488be5318dae198a69ce27fb31b");
182-
183181
private void SetNewBalance(Address address, in UInt256 balanceChange, IReleaseSpec releaseSpec, bool isSubtracting)
184182
{
185183
_needsStateRootUpdate = true;
186184

187-
if (address == importantAddress)
185+
if (address == FlatWorldStateScope.DebugAddress)
188186
{
189187
Console.Error.WriteLine($"Balance change {balanceChange}, {isSubtracting}");
190188
}
@@ -335,9 +333,9 @@ public byte[] GetCode(Address address)
335333

336334
public void DeleteAccount(Address address)
337335
{
338-
if (address == importantAddress)
336+
if (address == FlatWorldStateScope.DebugAddress)
339337
{
340-
Console.Error.WriteLine($"delete address");
338+
Console.Error.WriteLine($"delete address {Environment.StackTrace}");
341339
}
342340
_needsStateRootUpdate = true;
343341
PushDelete(address);
@@ -790,7 +788,13 @@ private void PushTouch(Address address, Account account, IReleaseSpec releaseSpe
790788
}
791789

792790
private void PushDelete(Address address)
793-
=> Push(address, null, ChangeType.Delete);
791+
{
792+
Push(address, null, ChangeType.Delete);
793+
if (address == FlatWorldStateScope.DebugAddress)
794+
{
795+
Console.Error.WriteLine("Delete account");
796+
}
797+
}
794798

795799
private void Push(Address address, Account? touchedAccount, ChangeType changeType)
796800
{
@@ -801,7 +805,7 @@ private void Push(Address address, Account? touchedAccount, ChangeType changeTyp
801805
return;
802806
}
803807

804-
if (address == importantAddress)
808+
if (address == FlatWorldStateScope.DebugAddress)
805809
{
806810
Console.Error.WriteLine($"Push {changeType}, {touchedAccount}");
807811
}
@@ -814,7 +818,7 @@ private void PushNew(Address address, Account account)
814818
StackList<int> stack = SetupCache(address);
815819
stack.Push(_changes.Count);
816820
_changes.Add(new Change(address, account, ChangeType.New));
817-
if (address == importantAddress)
821+
if (address == FlatWorldStateScope.DebugAddress)
818822
{
819823
Console.Error.WriteLine($"Push new {account}");
820824
}
@@ -825,7 +829,7 @@ private void PushRecreateEmpty(Address address, Account account, StackList<int>
825829
{
826830
stack.Push(_changes.Count);
827831
_changes.Add(new Change(address, account, ChangeType.RecreateEmpty));
828-
if (address == importantAddress)
832+
if (address == FlatWorldStateScope.DebugAddress)
829833
{
830834
Console.Error.WriteLine($"Push recreate empty {account}");
831835
}

0 commit comments

Comments
 (0)