Skip in plugins - #1093
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master-n3 #1093 +/- ##
=============================================
+ Coverage 51.49% 51.63% +0.13%
=============================================
Files 284 285 +1
Lines 16703 16742 +39
Branches 2149 2153 +4
=============================================
+ Hits 8602 8644 +42
+ Misses 7527 7526 -1
+ Partials 574 572 -2 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
roman-khimov
left a comment
There was a problem hiding this comment.
It's an improvement over the previous state, yet still an API change would be more efficient.
There was a problem hiding this comment.
Pull request overview
Adds a skip parameter to the storage Find/seek APIs used by plugins, enabling callers to efficiently skip N entries when iterating forward or backward. This aligns LevelDB and RocksDB plugin behavior and updates dependent package versions to a Neo build that includes the updated interface.
Changes:
- Extend
Find(...)in LevelDB/RocksDB storage plugins (and relevant tests) with an optionalskipargument. - Implement iterator skipping for RocksDB (via extension methods) and LevelDB (via iterator methods), wiring it into seek logic.
- Update Neo package references to
3.10.2-CI02099for CLI and plugins.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Neo.Plugins.ApplicationLogs.Tests/UT_NeoStore.cs | Updates tracking store/snapshot test wrappers to pass through the new skip argument. |
| tests/Neo.Cryptography.MPTTrie.Tests/Cryptography/MPTTrie/UT_Trie.cs | Updates the test snapshot stub’s Find signature to include skip. |
| src/Neo.CLI/Neo.CLI.csproj | Bumps Neo package reference to a version compatible with the updated persistence API. |
| plugins/RocksDBStore/Plugins/Storage/Store.cs | Adds skip to RocksDB store iteration and applies skipping after seek. |
| plugins/RocksDBStore/Plugins/Storage/Snapshot.cs | Adds skip to RocksDB snapshot iteration and applies skipping after seek. |
| plugins/RocksDBStore/Plugins/Storage/Iterator.cs | Introduces skip helper methods for RocksDB iterators. |
| plugins/LevelDBStore/Plugins/Storage/Store.cs | Adds skip to LevelDB store Find and forwards it to seek helper. |
| plugins/LevelDBStore/Plugins/Storage/Snapshot.cs | Adds skip to LevelDB snapshot Find and forwards it to seek helper. |
| plugins/LevelDBStore/IO/Data/LevelDB/Iterator.cs | Adds Skip/SkipPrev methods to the LevelDB iterator. |
| plugins/LevelDBStore/IO/Data/LevelDB/Helper.cs | Extends Seek helper to accept/apply skip for forward/backward iteration. |
| plugins/Directory.Build.props | Bumps Neo package reference for plugins to match the updated persistence API. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
I can't see any method to skip entries in the native libraries, could you give me a hint? |
|
There is none, what I mean is RPC API that triggers the need to skip. It shouldn't have any "skip" parameters, rather it should bring with it an initial offset. Then the first Seek moves us right in place. Much like in neo-project/neo#4039 |
|
@superboyiii merge? |
|
Wait for my final test today. |
| protected internal virtual JToken FindStorage(ContractNameOrHashOrId contractNameOrHashOrId, string base64KeyPrefix, int start = 0) | ||
| { | ||
| contractNameOrHashOrId.NotNull_Or(RpcError.InvalidParams.WithData($"Invalid 'contractNameOrHashOrId'")); | ||
| base64KeyPrefix.NotNull_Or(RpcError.InvalidParams.WithData($"Invalid 'base64KeyPrefix'")); |
There was a problem hiding this comment.
| base64KeyPrefix.NotNull_Or(RpcError.InvalidParams.WithData($"Invalid 'base64KeyPrefix'")); | |
| (start >= 0).True_Or(RpcError.InvalidParams.WithData($"Invalid 'start': {start}")); |
Previously FindStorage used LINQ .Skip(start), which treats a negative start as zero and still returns a successful page. After switching to FindContractStorage(..., skip: start), the same call throws ArgumentOutOfRangeException, which RpcServer maps to a generic HResult error instead of InvalidParams. Adding an explicit start >= 0 check keeps the stricter rejection, but returns the same clean InvalidParams response used for other bad RPC arguments.
|
|
||
| <ItemGroup> | ||
| <PackageReference Include="rocksdb" Version="10.10.1.1747" /> | ||
| <PackageReference Include="rocksdb" Version="11.8.1.4423" /> |
There was a problem hiding this comment.
Why is the RocksDB update needed?
No description provided.