-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Fixed
] DataCache Find Method
#3776
[Fixed
] DataCache Find Method
#3776
Conversation
@roman-khimov I reverted back that change. Also @superboyiii tested data on |
Co-authored-by: nan01ab <yjcc201374@outlook.com>
Mainnet data show it's compatible till now, but to ensure everything is the same, if behaviour is different, hardfork flag is necessary. |
Reverted Reused |
throw new ArgumentOutOfRangeException(nameof(keyOrPrefix)); | ||
|
||
var lastKey = new byte[ApplicationEngine.MaxStorageKeySize]; | ||
Array.Fill<byte>(lastKey, 0xff); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding this. I understand what you're talking about, but I've failed to reproduce the problem, see master...roman-khimov:neo:level-fun
Please tell me how to make it fail with the current code. It was built (d6620cb/#2819) around this snippet in fact: https://github.com/syndtr/goleveldb/blob/v1.0.0/leveldb/util/range.go#L20 (https://pkg.go.dev/github.com/syndtr/goleveldb@v1.0.0/leveldb#DB.NewIterator is also relevant here).
And the intention is exactly to get to the next item and make a step back which completely solves this 0xff
problem, you don't need to guess how many 0xff
you need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hope this helps shows the issue that I'm fixing
In this PR I already reverted MyFindRange method's functionality, but i can add it back if you would like.
// myDataCache.Find() and myDataCache.FindRange() uses old way
// MyFind() and MyFindRange() uses new way
var dateTime = new DateTime(2025, 01, 01);
var timeStamp1 = (ulong)new DateTimeOffset(dateTime).ToUnixTimeMilliseconds();
var timeStamp2 = (ulong)new DateTimeOffset(dateTime.AddDays(1)).ToUnixTimeMilliseconds();
var recordId = 8888u;
StorageKey key1 = new KeyBuilder(int.MaxValue, 0xff)
.AddBigEndian(timeStamp1) // Timestamp
.AddBigEndian(recordId); // Record Id
StorageKey key2 = new KeyBuilder(int.MaxValue, 0xff)
.AddBigEndian(timeStamp1) // Timestamp
.AddBigEndian(recordId + 1); // Record Id
StorageKey key3 = new KeyBuilder(int.MaxValue, 0xff)
.AddBigEndian(timeStamp2) // Timestamp
.AddBigEndian(recordId + 2); // Record Id
StorageKey key4 = new KeyBuilder(int.MaxValue, 0xff)
.AddBigEndian(timeStamp2) // Timestamp
.AddBigEndian(recordId + 3); // Record Id
myDataCache.Add(key1, value1);
myDataCache.Add(key2, value1);
myDataCache.Add(key3, value1);
myDataCache.Add(key4, value1);
// Used to get all records for start date 01/01/2025
var startSearchKey = new KeyBuilder(int.MaxValue, 0xff)
.AddBigEndian(timeStamp1)
.ToArray();
// Used to get all records for end date 01/01/2026
var endSearchKey = new KeyBuilder(int.MaxValue, 0xff)
.AddBigEndian(timeStamp2)
.AddBigEndian(uint.MaxValue) // Biggest Record Id
.ToArray();
byte[] keyId = [0xff, 0xff,]; // Contract Id
var action = () => _ = myDataCache.Find(keyId, SeekDirection.Backward); // Get All records of Contract with Id int.MaxValue
Assert.ThrowsException<ArgumentException>(action); // Breaks your Find() method and could leave records leftover or missed; Example ContractManagement.Destory
// Get all records for contract with id of int.MaxValue
var results = MyFind(keyId, SeekDirection.Backward); // Get All records of Contract with Id int.MaxValue
Assert.AreEqual(4, results.Count());
// Doesn't work in descending order
results = myDataCache.FindRange(startSearchKey, endSearchKey, SeekDirection.Backward); // Get records in descending order
Assert.AreEqual(0, results.Count());
// Workaround to get in descending order
results = myDataCache.FindRange(endSearchKey, startSearchKey, SeekDirection.Backward); // Get records in descending order
Assert.AreEqual(4, results.Count());
// New Method fixes start and end with backwards searching based off description of method
results = MyFindRange(startSearchKey, endSearchKey, SeekDirection.Backward); // Get records in descending order
Assert.AreEqual(4, results.Count());
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
var action = () => _ = myDataCache.Find(keyId, SeekDirection.Backward); // Get All records of Contract with Id int.MaxValue
Assert.ThrowsException<ArgumentException>(action); // Breaks your Find() method and could leave records leftover or missed; Example ContractManagement.Destory
Not a problem practically, these are evaluated in natural order.
// Workaround to get in descending order
results = myDataCache.FindRange(endSearchKey, startSearchKey, SeekDirection.Backward); // Get records in descending order
Assert.AreEqual(4, results.Count());
That's just the way FindRange()
works. And there is code relying on this behavior in NEO and Role Management native contracts at least.
So, what exactly are we trying to fix here?
Also, regarding 0xff
--- it's not needed and it doesn't improve anything, current code is correct since your initial seek for backwards iteration will routinely overshoot past the desired prefix:
// Position at the first key in the source that is at or past target.
// The iterator is Valid() after this call iff the source contains
// an entry that comes at or past target.
virtual void Seek(const Slice& target) = 0;
Description
As you may or may not know,
Find
does not return all records within range. AlsoFind
wouldn't allow and build the last key correctly. Now the ranges and last search key are built and returned correctly.Change Log
Find
now builds the correct key ending.Find
now return all the records within the range (Including thestart
andend
key).TestFindRange
inUT_DataCache
class. (to check final return of records).Type of change
How Has This Been Tested?
Checklist: