Change origin hash to 32 bytes #3120
Conversation
src/internet_identity/src/storage.rs
Outdated
| // Check that the memory is empty, otherwise panic | ||
| assert_eq!(self.lookup_application_with_origin_memory.len(), 0); |
There was a problem hiding this comment.
Given that running below insert is a noop, I don't think we need this guard.
There was a problem hiding this comment.
I interpret this as meaning that it does not destroy anything - true, I had this in so we definitely don't forget to remove it afterwards, but I can also take out the check.
There was a problem hiding this comment.
Could we have a short document of how we expect to do this migration.
This is touching the storage layer and it's important because it uses post_upgrade, which if we mess up we are very much screwed.
Therefore, I think that a short design doc explaining what we want to do is time well spent.
| const LOOKUP_APPLICATION_WITH_ORIGIN_MEMORY_INDEX: u8 = 12u8; | ||
| // This memory index has been abandoned, do not use it | ||
| // const LOOKUP_APPLICATION_WITH_ORIGIN_MEMORY_INDEX: u8 = 12u8; | ||
| const LOOKUP_APPLICATION_WITH_ORIGIN_MEMORY_INDEX: u8 = 19u8; |
There was a problem hiding this comment.
Should we have this after the STABLE_ACCOUNT_COUNTER_DISCREPANCY_COUNTER_MEMORY_INDEX which is the 18 index?
Otherwise I can see ourselves adding the 19 again...
|
|
||
| /// Used for migrating from 8-byte to 32-byte origin hash | ||
| pub fn rebuild_lookup_application_with_origin_memory(&mut self) { | ||
| self.stable_application_memory |
There was a problem hiding this comment.
Won't this be pointing to the new stable memory with index 19?
We need to keep both accessible until we finish the migration, right?
There was a problem hiding this comment.
self.stable_application_memory points to memory ID 11. We only need to rebuild the lookup table, as it is the only place the origin hash is used. The old lookup table can be discarded.
Motivation
To address ProdSec finding F01, we are changing the OriginHash to be the full 32-bytes. This is a draft because we'd still need to address memory migration.
Changes
Change the StorableOriginHash to 32 bytes, update its functions accordingly.
Tests
No new tests were added.