Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 11 additions & 8 deletions plugins/LevelDBStore/Plugins/Storage/Snapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ internal class Snapshot : IStoreSnapshot, IEnumerable<KeyValuePair<byte[], byte[
{
private readonly DB _db;
private readonly LSnapshot _snapshot;
private readonly ReadOptions _readOptions;
private readonly ReadOptions _scanReadOptions;
private readonly ReadOptions _pointReadOptions;
private readonly WriteBatch _batch;
private readonly Lock _lock = new();

Expand All @@ -36,7 +37,8 @@ internal Snapshot(Store store, DB db)
Store = store;
_db = db;
_snapshot = db.CreateSnapshot();
_readOptions = new ReadOptions { FillCache = false, Snapshot = _snapshot };
_scanReadOptions = new ReadOptions { FillCache = false, Snapshot = _snapshot };
_pointReadOptions = new ReadOptions { Snapshot = _snapshot };
_batch = new WriteBatch();
}

Expand Down Expand Up @@ -64,35 +66,36 @@ public void Put(byte[] key, byte[] value)
public void Dispose()
{
_snapshot.Dispose();
_readOptions.Dispose();
_scanReadOptions.Dispose();
_pointReadOptions.Dispose();
_batch.Dispose();
}

/// <inheritdoc/>
public IEnumerable<(byte[] Key, byte[] Value)> Find(byte[]? keyOrPrefix, SeekDirection direction = SeekDirection.Forward)
{
return _db.Seek(_readOptions, keyOrPrefix, direction);
return _db.Seek(_scanReadOptions, keyOrPrefix, direction);
}

public bool Contains(byte[] key)
{
return _db.Contains(_readOptions, key);
return _db.Contains(_pointReadOptions, key);
}

public byte[]? TryGet(byte[] key)
{
return _db.Get(_readOptions, key);
return _db.Get(_pointReadOptions, key);
}

public bool TryGet(byte[] key, [NotNullWhen(true)] out byte[]? value)
{
value = _db.Get(_readOptions, key);
value = _db.Get(_pointReadOptions, key);
return value != null;
}

public IEnumerator<KeyValuePair<byte[], byte[]>> GetEnumerator()
{
using var iterator = _db.CreateIterator(_readOptions);
using var iterator = _db.CreateIterator(_scanReadOptions);
for (iterator.SeekToFirst(); iterator.Valid(); iterator.Next())
yield return new KeyValuePair<byte[], byte[]>(iterator.Key()!, iterator.Value()!);
}
Expand Down
20 changes: 12 additions & 8 deletions plugins/RocksDBStore/Plugins/Storage/Snapshot.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ internal class Snapshot : IStoreSnapshot
private readonly RocksDb _db;
private readonly RocksDbSharp.Snapshot _snapshot;
private readonly WriteBatch _batch;
private readonly ReadOptions _options;
private readonly ReadOptions _scanOptions;
private readonly ReadOptions _pointOptions;
private readonly Lock _lock = new();

public IStore Store { get; }
Expand All @@ -35,9 +36,12 @@ internal Snapshot(Store store, RocksDb db)
_snapshot = db.CreateSnapshot();
_batch = new WriteBatch();

_options = new ReadOptions();
_options.SetFillCache(false);
_options.SetSnapshot(_snapshot);
_scanOptions = new ReadOptions();
_scanOptions.SetFillCache(false);
_scanOptions.SetSnapshot(_snapshot);

_pointOptions = new ReadOptions();
_pointOptions.SetSnapshot(_snapshot);
}

public void Commit()
Expand All @@ -63,7 +67,7 @@ public void Put(byte[] key, byte[] value)
{
keyOrPrefix ??= [];

using var it = _db.NewIterator(readOptions: _options);
using var it = _db.NewIterator(readOptions: _scanOptions);

if (direction == SeekDirection.Forward)
for (it.Seek(keyOrPrefix); it.Valid(); it.Next())
Expand All @@ -75,17 +79,17 @@ public void Put(byte[] key, byte[] value)

public bool Contains(byte[] key)
{
return _db.Get(key, Array.Empty<byte>(), 0, 0, readOptions: _options) >= 0;
return _db.Get(key, Array.Empty<byte>(), 0, 0, readOptions: _pointOptions) >= 0;
}

public byte[]? TryGet(byte[] key)
{
return _db.Get(key, readOptions: _options);
return _db.Get(key, readOptions: _pointOptions);
}

public bool TryGet(byte[] key, [NotNullWhen(true)] out byte[]? value)
{
value = _db.Get(key, readOptions: _options);
value = _db.Get(key, readOptions: _pointOptions);
return value != null;
}

Expand Down
102 changes: 102 additions & 0 deletions tests/Neo.Plugins.Storage.Tests/SnapshotReadOptionsTest.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Copyright (C) 2015-2026 The Neo Project.
//
// SnapshotReadOptionsTest.cs file belongs to the neo project and is free
// software distributed under the MIT software license, see the
// accompanying file LICENSE in the main directory of the
// repository or http://www.opensource.org/licenses/mit-license.php
// for more details.
//
// Redistribution and use in source and binary forms with or without
// modifications are permitted.

using System.Reflection;

namespace Neo.Plugins.Storage.Tests;

[TestClass]
public class SnapshotReadOptionsTest
{
[TestMethod]
public void LevelDbSnapshotUsesDedicatedReadOptionsForScanAndPointReads()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[suggestion] LevelDbSnapshotUsesDedicatedReadOptionsForScanAndPointReads and RocksDbSnapshotUsesDedicatedReadOptionsForScanAndPointReads never construct a snapshot and never inspect fill-cache. They only scan method IL for the _scan* vs _point* field tokens. A constructor that set FillCache=false on the point-read options (or forgot SetFillCache(false) on the scan options) would still pass, which is the behavior this PR is meant to lock in. UsesField also matches a metadata token anywhere in the body (no ldfld/ldflda check) and AssertMethodUsesOnly treats “any overload/state-machine method references the expected field” as success, so one TryGet overload could silently miss the point-read options if the other hit.

Suggestion: Also scan the constructors (or otherwise assert setup) so scan options are the only ones that disable fill-cache, and require every TryGet overload to load the point-read field. Prefer opcode-aware ldfld matching over a raw 4-byte token search.

{
var snapshotType = GetSnapshotType(typeof(LevelDBStore));
var scanReadOptions = GetField(snapshotType, "_scanReadOptions");
var pointReadOptions = GetField(snapshotType, "_pointReadOptions");

AssertMethodUsesOnly(snapshotType, "Find", scanReadOptions, pointReadOptions);
AssertMethodUsesOnly(snapshotType, "GetEnumerator", scanReadOptions, pointReadOptions);
AssertMethodUsesOnly(snapshotType, "Contains", pointReadOptions, scanReadOptions);
AssertMethodUsesOnly(snapshotType, "TryGet", pointReadOptions, scanReadOptions);
}

[TestMethod]
public void RocksDbSnapshotUsesDedicatedReadOptionsForScanAndPointReads()
{
var snapshotType = GetSnapshotType(typeof(RocksDBStore));
var scanOptions = GetField(snapshotType, "_scanOptions");
var pointOptions = GetField(snapshotType, "_pointOptions");

AssertMethodUsesOnly(snapshotType, "Find", scanOptions, pointOptions);
AssertMethodUsesOnly(snapshotType, "Contains", pointOptions, scanOptions);
AssertMethodUsesOnly(snapshotType, "TryGet", pointOptions, scanOptions);
}

private static Type GetSnapshotType(Type storeProviderType)
{
return storeProviderType.Assembly.GetType("Neo.Plugins.Storage.Snapshot", throwOnError: true)!;
}

private static FieldInfo GetField(Type type, string fieldName)
{
var field = type.GetField(fieldName, BindingFlags.Instance | BindingFlags.NonPublic);
Assert.IsNotNull(field, $"{type.FullName} should declare {fieldName}.");
return field;
}

private static void AssertMethodUsesOnly(Type type, string methodName, FieldInfo expectedField, FieldInfo unexpectedField)
{
var implementations = GetImplementations(type, methodName).ToArray();
Assert.IsNotEmpty(implementations, $"{type.FullName}.{methodName} should exist.");
Assert.IsTrue(
implementations.Any(method => UsesField(method, expectedField)),
$"{type.FullName}.{methodName} should use {expectedField.Name}.");
Assert.IsFalse(
implementations.Any(method => UsesField(method, unexpectedField)),
$"{type.FullName}.{methodName} should not use {unexpectedField.Name}.");
}

private static IEnumerable<MethodBase> GetImplementations(Type type, string methodName)
{
const BindingFlags flags = BindingFlags.Instance | BindingFlags.Public | BindingFlags.NonPublic;

foreach (var method in type.GetMethods(flags).Where(method => method.Name == methodName))
yield return method;

foreach (var nestedType in type.GetNestedTypes(BindingFlags.NonPublic))
{
if (!nestedType.Name.Contains($"<{methodName}>")) continue;

var moveNext = nestedType.GetMethod("MoveNext", flags);
if (moveNext != null)
yield return moveNext;
}
}

private static bool UsesField(MethodBase method, FieldInfo field)
{
var body = method.GetMethodBody()?.GetILAsByteArray();
if (body is null) return false;

var token = BitConverter.GetBytes(field.MetadataToken);
for (var i = 0; i <= body.Length - token.Length; i++)
{
if (body[i] == token[0]
&& body[i + 1] == token[1]
&& body[i + 2] == token[2]
&& body[i + 3] == token[3])
return true;
}

return false;
}
}
Loading