chore(sequencer)!: update storage keys locations and values (ENG-898)#1616
chore(sequencer)!: update storage keys locations and values (ENG-898)#1616
Conversation
SuperFluffy
left a comment
There was a problem hiding this comment.
This is great, thank you for going through all the keys and streamlining them.
I very much like that we have full (I think?) coverage of all keys and prefixes.
Since this is a breaking change and a rework of how our fees are structured, I think we should break with our previous key and make their types and (where applicable) byte-encoding consistent everywhere.
Some open points that I think should be addressed before merge:
- make the type of all keys consistently
&str/String. Right now we have a string representation in some places, and raw byte slices in others. It's not clear why that's done. Yet especially for snapshot tests it's very valuable to have a human readable representation. - Apart from ibc-prefixed assets (which are
"ibc/<hex-encoded-32-bytes>", all key/prefix parts that involve raw bytes (like account) should be base64 encoded to save space. - I don't have a strong opinion on only using the byte part of ibc-prefixed assets when constructing keys from them. I mildly lean toward encoding the full string (including
"ibc/") and not just the bytes.
SuperFluffy
left a comment
There was a problem hiding this comment.
This looks great, thank you.
I wonder if in this PR we should just change base64 encoding to the url safe alphabet everywhere (so that we don't have a different encoding in the storage keys and the normal display impl).
Ticketed for follow-up at #1649. |
* main: feat(sequencer)!: enforce block ordering by transaction group (#1618) fix(cli): ensure checkTx passes before waiting for inclusion (#1636) chore(sequencer)!: update storage keys locations and values (ENG-898) (#1616) chore: cargo audit warning (#1644) chore(proto, core)!: remove action suffix from all action types (#1630) feat(sequencer)!: add limit to total amount of transactions in parked (#1638)
Summary
The storage keys for all components were moved from the top of
state_extmodules into newstorage::keysmodules. The key values were updated to a consistent style.Background
We wanted the keys to be easy to find and to follow a consistent pattern in terms of their values and locations.
Changes
<COMPONENT>/storage/keys./._as word separators.state_extmodules for formatting keys were unified into a new structAddressPrefixerincrate::storage.Assetfunctionality was moved fromcrate::storage_keystocrate::storage.Testing
state_extmodules are tested to ensure they are actually prefixes of the given keys.Breaking Changelist
Related Issues
Closes #1611.