Bug fix: StateMap::Keys are not consistent across platforms

[preston-evans98]

Description

This PR removes our StateMap key encoding based on std::Hash, which was not consistent across platforms. This caused the demo-prover to fail, since the generated witness used different storage keys than the guest execution. To fix this issue without sacrificing flexibility. PR introduces the EncodeLike<Ref, Target> trait which marks that Ref can be encoded like Target by the implementing codec.

This PR also removes the SingletonKey type, which required special handling in codecs. Instead, of using this placeholder, this PR implements auxiliary methods on the working set for dealing with singletons.

Update: September 07, 2023

After discussion with @neysofu, we decided that it was desirable to allow implementers to use different codecs for keys and values.

The new design is as follows:

pub trait StateCodec {
    type KeyCodec;
    type ValueCodec;

    fn key_codec(&self) -> &Self::KeyCodec;
    fn value_codec(&self) -> &Self::ValueCodec;
}

pub trait StateKeyCodec<K> {
    fn encode_key(&self, key: &K) -> Vec<u8>;
}

pub trait StateValueCodec<V> {
    type Error: std::fmt::Debug;
    fn encode_value(&self, value: &V) -> Vec<u8>;
    fn try_decode_value(&self, bytes: &[u8]) -> Result<V, Self::Error>;
}

pub trait EncodeKeyLike<Ref: ?Sized, Target> {
    fn encode_key_like(&self, borrowed: &Ref) -> Vec<u8>;
}

All three of our state storage types (StateMap, StateKey, and StateVec) are defined with an associated StateCodec, which delegates to its own KeyCodec and ValueCodec as appropriate. This allows us to avoid introducing an extra generic in the State* types, while allowing separate key and value codecs.

This PR also introduces the SplitCodec<KC, VC> type which allows anyone to combine a KeyCodec and ValueCodec into a StateCodec. The current trait design allows this SplitCodec to directly inherit the functionality of its associated key and value codecs rather. Without the StateCodec trait, this would be not be feasible. Instead, SplitCodec would have to try to blanket impl the KeyCodec and ValueCodec traits, which is not always possible without specialization.

Future Work

This design leaves room for an additional EncodeValueLike trait to be implemented in the future if desired. Such a trait would could allow the storage of borrowed types, potentially removing some unnecessary clones.

Linked Issues

• Fixes # (issue, if applicable)
• Related to # (issue)

Testing

I ran the demo-rollup before on the same input before and after introducing this fix:

Before

2023-09-06T22:34:33.069691Z  INFO sov_demo_prover_host: Completed proving and verifying block 2
2023-09-06T22:34:33.069709Z  INFO sov_demo_prover_host: Requesting data for height 3 and prev_state_root 0xa5ee72743963a012e754215fbd135ccd4d1a75915fb7044d605df6fa305c4926
2023-09-06T22:34:33.087281Z  INFO sov_demo_prover_host: Extracted 1 relevant blobs at height 3 header 0x1e90259c5a48c6dd20f1dd190bc21381e0ba8e9a26f833ec5d2845a79843f059
2023-09-06T22:34:33.087338Z  INFO sov_modules_stf_template: Selected 1 blob(s) for execution in current slot
2023-09-06T22:34:33.087433Z  INFO sov_modules_stf_template: blob #0 from sequencer celestia1w7wcupk5gswj25c0khnkey5fwmlndx6t5aarmk with blob_hash 0x24939693604755bf82bcdcda7682be007e639899b82341a98a61ab3077468f6e has been applied with #1 transactions, sequencer outcome Rewarded(0)
2023-09-06T22:34:33.087438Z  INFO sov_modules_stf_template: tx #0 hash: 0x1d0d746984613c0874fd2515d5de82e717c3705284f00ea8fecda756feadabed result Successful

Start guest
Prev root hash read
header has been read
inclusion proof has been read
!completeness proof has been read
blobs have been read
Witness have been read
Applying slot...
thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `0`,
 right: `252`: Batch deserialization failed and some data was not provided. The prover might be malicious', ./src/app_template.rs:270:17
thread 'main' panicked at 'Prover should run successfully: illegal instruction at 0x839e4: 11000000000000000001000001110011 (1110011, 001, 00000, 1100000)', host/src/main.rs:142:18
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

After

2023-09-06T22:40:18.793261Z  INFO sov_demo_prover_host: Completed proving and verifying block 2
2023-09-06T22:40:18.793279Z  INFO sov_demo_prover_host: Requesting data for height 3 and prev_state_root 0x15f8d68a89fd36c8dd22058a1a0becf12f27791c74a59a6fdbb55bcc533837f3
2023-09-06T22:40:18.813870Z  INFO sov_demo_prover_host: Extracted 1 relevant blobs at height 3 header 0x1e90259c5a48c6dd20f1dd190bc21381e0ba8e9a26f833ec5d2845a79843f059
2023-09-06T22:40:18.813901Z  INFO sov_modules_stf_template: Selected 1 blob(s) for execution in current slot
2023-09-06T22:40:18.813989Z  INFO sov_modules_stf_template: blob #0 from sequencer celestia1w7wcupk5gswj25c0khnkey5fwmlndx6t5aarmk with blob_hash 0x24939693604755bf82bcdcda7682be007e639899b82341a98a61ab3077468f6e has been applied with #1 transactions, sequencer outcome Rewarded(0)
2023-09-06T22:40:18.813994Z  INFO sov_modules_stf_template: tx #0 hash: 0x1d0d746984613c0874fd2515d5de82e717c3705284f00ea8fecda756feadabed result Successful

Start guest
Prev root hash read
header has been read
inclusion proof has been read
!completeness proof has been read
blobs have been read
Relevant txs verified
Witness have been read
Applying slot...
Slot has been applied
new state root committed

Docs

Describe where this code is documented. If it changes a documented interface, have the docs been updated?

[codecov]

Codecov Report

Merging #804 (2fb8436) into nightly (9efc47a) will decrease coverage by 0.1%.
The diff coverage is 81.8%.

Files Changed	Coverage Δ
module-system/sov-state/src/codec/bcs_codec.rs	6.2% <0.0%> (-8.1%)	⬇️
module-system/sov-state/src/codec/split_codec.rs	0.0% <0.0%> (ø)
rollup-interface/src/state_machine/optimistic.rs	0.0% <ø> (ø)
...e-system/sov-state/src/containers/accessory_map.rs	50.0% <53.8%> (-3.9%)	⬇️
...system/sov-state/src/containers/accessory_value.rs	39.5% <80.0%> (-1.3%)	⬇️
module-system/sov-state/src/containers/map.rs	91.4% <89.4%> (-2.0%)	⬇️
module-system/sov-state/src/codec/borsh_codec.rs	100.0% <100.0%> (ø)
module-system/sov-state/src/codec/json_codec.rs	100.0% <100.0%> (ø)
module-system/sov-state/src/codec/mod.rs	81.4% <100.0%> (+26.9%)	⬆️
...e-system/sov-state/src/containers/accessory_vec.rs	86.6% <100.0%> (ø)
... and 4 more

... and 1 file with indirect coverage changes

📢 Have feedback on the report? Share it here.

[vlopes11]

Looks good! I left two comments that are more potential future improvements than something to be done now

[neysofu]

Looks good, but it also removes support for having different codecs for keys and values by using StateValueCodec everywhere, which can't differentiate between keys and values.

I left a few other minor comments, otherwise LGTM!