Refactor Witness trait so it only require serde Serialize/Deserialize and manages hints differently

[citizen-stig]

Follow up of #260

For now blocked by https://github.com/penumbra-zone/jmt/pull/92

Trait

Currently Witness trait that is defined inside sov-state module have this defintion:

pub trait Witness: Default + Serialize {
    fn add_hint<T: BorshSerialize>(&self, hint: T);
    fn get_hint<T: BorshDeserialize>(&self) -> T;
    fn merge(&self, rhs: &Self);
}

Which mixes serde and borsh in single trait. Final goal for it to have only serde and probably change methods, so it can accept serializable hints and whole serialization of the witness can be in one go, something like that:

pub trait Witness: Default + Serialize  + DeserializeOwned {
    fn add_hint<T: Serialize + DeserializeOwned>(&self, hint: T);
    fn get_hint<T: Serialize + DeserializeOwned>(&self) -> T;
    fn merge(&self, rhs: &Self);
}

Implementers

and currently we only have ArrayWitness:

#[derive(Default, Debug, Serialize, Deserialize)]
pub struct ArrayWitness {
    next_idx: AtomicUsize,
    hints: RefCell<Vec<Vec<u8>>>,
}

impl Witness for ArrayWitness {
    fn add_hint<T: BorshSerialize>(&self, hint: T) {
        todo!("serializes immediatly into Vec<u8>");
    }

    fn get_hint<T: BorshDeserialize>(&self) -> T {
        todo!("deserializes immediatly into from vec");
    }

    fn merge(&self, rhs: &Self) {
        let rhs_next_idx = rhs.next_idx.load(std::sync::atomic::Ordering::SeqCst);
        self.hints
            .borrow_mut()
            .extend(rhs.hints.borrow_mut().drain(rhs_next_idx..))
    }
}

Which could be changed to something like this:

use jmt::proof::SparseMerkleProof;
use jmt::storage::{Node};
use jmt::SimpleHasher;
use serde::{Serialize,Deserialize};

#[derive(Default, Debug, Serialize, Deserialize, BorshSerialize, BorshDeserialize)]
struct ArrayWitnessNew<H: SimpleHasher> {
    versions: Vec<u64>,
    values: Vec<StorageValue>,
    nodes: Vec<Node>,
    proofs: Vec<SparseMerkleProof<H>>,
}

trait ArrayWitnessNewTrait<H: SimpleHasher>: Default + Serialize + DeserializeOwned {
    fn add_version(&mut self, version: u64);
    fn get_version(&self, idx: usize) -> u64;
    fn add_value(&mut self, value: StorageValue);
    fn get_value(&self, idx: usize) -> StorageValue;
    fn add_node(&mut self, node: Node);
    fn get_node(&self, idx: usize) -> Node;
    fn add_proof(&mut self, proof: SparseMerkleProof<H>);
    fn get_proof(&self, idx: usize) -> SparseMerkleProof<H>;
    fn merge(&self, rhs: &Self);
}

[citizen-stig]

I mainly discussed it with @preston-evans98 , please take a look.

Also would be nice to have input from @bkolad @dubbelosix

[preston-evans98]

Yep, looks good for now! At some point, #246 will get fixed and remove the need to pass a Versions and jmt::Nodes - but this is great for now