feat: Implement caching layer for LazyOption #444
Conversation
near-sdk/src/store/lazy/mod.rs
Outdated
| { | ||
| let cache = match value { | ||
| Some(value) => CacheEntry::new_modified(Some(value)), | ||
| None => CacheEntry::new_cached(None), |
There was a problem hiding this comment.
Nothing to change here I don't think, but just want to note that in the case that the storage value already exists, this will not clear the key/value and this could lead to a corrupted state. Could potentially be a foot gun for a developer not aware of the internals of this.
Does env::storage_remove charge gas if there was no value at that KV previously, @evgenykuzyakov?
There was a problem hiding this comment.
According to near-vm-logic/src/logic.rs, it does charge gas and it looks to be the same amount of gas if the value were present in storage.
But this begs the question about how we should handle already stored values when creating new LazyOptions. Right now, doing LazyOption::new(key, Some(val)) would effectively replace the value in storage, while LazyOption::new(key, None) doesn't do anything and allows LazyOption::get to retrieve the value from storage. This behavior is the same with collections::LazyOption.
If we have LazyOption::new(key, None) be similar to replacing/deleting the value in storage, then we might need a new way to create a LazyOption where it doesn't do anything and we can call LazyOption::get.
There was a problem hiding this comment.
yeah I think it's fine for now, and we can just think through any edge cases before we stabilize this!
|
oh, and would you mind adding a |
| assert_eq!(a.get(), &Some(49)); | ||
|
|
||
| // Testing `Option::replace` | ||
| let old = a.replace(69u32); |
This addresses #439 and implements a caching layer for
LazyOption, but the oldcollections::LazyOptionis still there since not all interfaces are the same and the newstore::LazyOptionisunstable.LazyOptionis pretty similar code-wise to theLazytype. So we could just end up usingLazyOptionto implementLazysince we could just unwrap all theOptions theLazyOptionreturns.