sealable-trie: don’t use varint for tag

Using varints for the length of proof isn’t worth it in the end.  The
proof will never be longer than 8192 elements (longest possible key
is 512 bytes long) and even with the additional bit for distinguishing
membership from non-membership proofs 16-bits is enough to encode it.

Using varint to encode the tag saves us at most only one byte at the
cost of having bunch of convoluted code.

common/lib/src/lib.rs

@@ -7,4 +7,3 @@ extern crate std;
 pub mod hash;
 #[cfg(any(feature = "test_utils", test))]
 pub mod test_utils;
-pub mod varint;

common/sealable-trie/src/proof/serialisation.rs

@@ -8,13 +8,14 @@ use core::num::NonZeroU16;
 use borsh::maybestd::io;
 use borsh::{BorshDeserialize, BorshSerialize};
 use lib::hash::CryptoHash;
-use lib::varint::VarInt;
 #[cfg(test)]
 use pretty_assertions::assert_eq;
 
 use super::{Actual, Item, OwnedRef, Proof};
 
-// Encoding: <varint((items.len() + has_actual) * 2 + is_non_membership)>
+const NON_MEMBERSHIP_SHIFT: u32 = 15;
+
+// Encoding: <(items.len() + has_actual + (is_non_membership << 15)) as u16>
 //           <actual>? <item>*
 impl BorshSerialize for Proof {
     fn serialize<W: io::Write>(&self, wr: &mut W) -> io::Result<()> {
@@ -24,13 +25,15 @@ impl BorshSerialize for Proof {
         };
 
         debug_assert!(!membership || actual.is_none());
-        let tag = items
-            .len()
-            .checked_add(usize::from(actual.is_some()))
-            .and_then(|tag| tag.checked_mul(2))
-            .and_then(|tag| u32::try_from(tag + usize::from(!membership)).ok())
-            .unwrap();
-        VarInt(tag).serialize(wr)?;
+        u16::try_from(items.len())
+            .ok()
+            .and_then(|tag| tag.checked_add(u16::from(actual.is_some())))
+            .filter(|tag| *tag < 8192)
+            .map(|tag| tag | (u16::from(!membership) << NON_MEMBERSHIP_SHIFT))
+            .ok_or_else(|| {
+                invalid_data(format!("proof too long: {}", items.len()))
+            })?
+            .serialize(wr)?;
         if let Some(actual) = actual {
             actual.serialize(wr)?;
         }
@@ -43,9 +46,9 @@ impl BorshSerialize for Proof {
 
 impl BorshDeserialize for Proof {
     fn deserialize_reader<R: io::Read>(rd: &mut R) -> io::Result<Self> {
-        let tag = VarInt::deserialize_reader(rd)?.0;
-        let is_membership = tag & 1 == 0;
-        let len = usize::try_from(tag / 2).unwrap();
+        let tag = u16::deserialize_reader(rd)?;
+        let is_membership = tag & (1 << NON_MEMBERSHIP_SHIFT) == 0;
+        let len = usize::from(tag & !(1 << NON_MEMBERSHIP_SHIFT));
 
         // len == 0 means there’s no Actual or Items.  Return empty proof.
         // (Note: empty membership proof never verifies but is valid as far as
@@ -459,36 +462,41 @@ fn test_proof_borsh() {
     let item = Item::Extension(NonZeroU16::new(42).unwrap());
     let actual = Actual::LookupKeyLeft(NonZeroU16::MIN, CryptoHash::test(1));
 
-    test(Proof::Positive(super::Membership(vec![])), &[0]);
-    test(Proof::Positive(super::Membership(vec![item.clone()])), &[2, 32, 42]);
+    test(Proof::Positive(super::Membership(vec![])), &[0, 0]);
+    test(Proof::Positive(super::Membership(vec![item.clone()])), &[
+        1, 0, 32, 42,
+    ]);
     test(
         Proof::Positive(super::Membership(vec![item.clone(), item.clone()])),
-        &[4, 32, 42, 32, 42],
+        &[2, 0, 32, 42, 32, 42],
     );
-    test(Proof::Negative(super::NonMembership(None, vec![])), &[1]);
+    test(Proof::Negative(super::NonMembership(None, vec![])), &[0, 0x80]);
     test(Proof::Negative(super::NonMembership(None, vec![item.clone()])), &[
-        3, 32, 42,
+        1, 0x80, 32, 42,
     ]);
+    #[rustfmt::skip]
     test(
         Proof::Negative(super::NonMembership(
             Some(actual.clone().into()),
             vec![],
         )),
         &[
-            /* proof tag: */ 3, /* actual: */ 134, 1, 0, 0, 0, 0, 1,
-            0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0,
-            0, 1, 0, 0, 0, 1,
+            /* proof tag: */ 1, 0x80,
+            /* actual: */ 134, 1, 0, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0,
+                          0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1,
         ],
     );
+    #[rustfmt::skip]
     test(
         Proof::Negative(super::NonMembership(
             Some(actual.clone().into()),
             vec![item.clone()],
         )),
         &[
-            /* proof tag: */ 5, /* actual: */ 134, 1, 0, 0, 0, 0, 1,
-            0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0,
-            0, 1, 0, 0, 0, 1, /* item: */ 32, 42,
+            /* proof tag: */ 2, 0x80,
+            /* actual: */ 134, 1, 0, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0,
+                          0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1, 0, 0, 0, 1,
+            /* item: */ 32, 42,
         ],
     );
 }