-
Notifications
You must be signed in to change notification settings - Fork 17
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
Implement contract state rent #32
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,93 @@ | ||
package sized | ||
|
||
import ( | ||
"io" | ||
|
||
"github.com/cosmos/cosmos-sdk/store/types" | ||
) | ||
|
||
var _ types.KVStore = &Store{} | ||
|
||
type Store struct { | ||
parent types.KVStore | ||
sizeChange int64 | ||
} | ||
|
||
func NewStore(parent types.KVStore) *Store { | ||
return &Store{parent: parent} | ||
} | ||
func (store *Store) GetWorkingHash() ([]byte, error) { | ||
return store.parent.GetWorkingHash() | ||
} | ||
|
||
func (s *Store) GetSizeChanged() int64 { | ||
return s.sizeChange | ||
} | ||
|
||
func (s *Store) Get(key []byte) []byte { | ||
value := s.parent.Get(key) | ||
return value | ||
} | ||
|
||
func (s *Store) Set(key []byte, value []byte) { | ||
oldValue := s.Get(key) | ||
if oldValue != nil { | ||
// reduce size due to overwrite | ||
s.sizeChange -= int64(len(key)) | ||
s.sizeChange -= int64(len(oldValue)) | ||
} | ||
s.parent.Set(key, value) | ||
if key != nil { | ||
s.sizeChange += int64(len(key)) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. do we care about key size? i thought the limiting factor on performance for keys was key cardinality due to tree traversal operational time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we care about key size for the storage space it consumes |
||
} | ||
if value != nil { | ||
s.sizeChange += int64(len(value)) | ||
} | ||
} | ||
|
||
func (s *Store) Delete(key []byte) { | ||
// has to perform a read here to know the size change | ||
value := s.Get(key) | ||
s.parent.Delete(key) | ||
if value != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is it possible for a key to be set with a nil value (such that the key existed previously). in this case, we would still need to decrement for keyLen bytes, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it's not possible and validated here https://github.com/sei-protocol/sei-cosmos/blob/main/store/cachekv/store.go#L132 |
||
// only reduce size if the key used to have a value | ||
s.sizeChange -= int64(len(key)) | ||
s.sizeChange -= int64(len(value)) | ||
} | ||
} | ||
|
||
func (s *Store) Has(key []byte) bool { | ||
return s.parent.Has(key) | ||
} | ||
|
||
func (s *Store) Iterator(start, end []byte) types.Iterator { | ||
return s.parent.Iterator(start, end) | ||
} | ||
|
||
func (s *Store) ReverseIterator(start, end []byte) types.Iterator { | ||
return s.parent.ReverseIterator(start, end) | ||
} | ||
|
||
// GetStoreType implements the KVStore interface. It returns the underlying | ||
// KVStore type. | ||
func (s *Store) GetStoreType() types.StoreType { | ||
return s.parent.GetStoreType() | ||
} | ||
|
||
// CacheWrap implements the KVStore interface. It panics as a Store | ||
// cannot be cache wrapped. | ||
func (s *Store) CacheWrap(_ types.StoreKey) types.CacheWrap { | ||
panic("cannot CacheWrap a ListenKVStore") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just curious, what is the normal behavior of cachewrap, and why do we want it to panic here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's used for store types that need the ability the "branch out" on that particular inheritance level. Unfortunately a lot of functions' parameter are using the interface that includes these methods. Panicing for wrapper store types that don't need branching at their inheritance levels seem to be common practice for types like gaskv/listenkv/tracekv |
||
} | ||
|
||
// CacheWrapWithTrace implements the KVStore interface. It panics as a | ||
// Store cannot be cache wrapped. | ||
func (s *Store) CacheWrapWithTrace(_ types.StoreKey, _ io.Writer, _ types.TraceContext) types.CacheWrap { | ||
panic("cannot CacheWrapWithTrace a ListenKVStore") | ||
} | ||
|
||
// CacheWrapWithListeners implements the KVStore interface. It panics as a | ||
// Store cannot be cache wrapped. | ||
func (s *Store) CacheWrapWithListeners(_ types.StoreKey, _ []types.WriteListener) types.CacheWrap { | ||
panic("cannot CacheWrapWithListeners a ListenKVStore") | ||
} |
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.
i imagine this would be such a small value, would it be more worth it to make this param per
10kb
per block or something and then have the unit price represented as an int (similar to how the wasm vm gas multiplier is represented)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.
we can do price per 10kb or more but I'd prefer keeping the type as dec for flexibility just in case in the future we want to decrease price by a lot