[authority] Small code fixes on the back of profiling

[gdanezis]

• Cache the certificate digest but do not serialize / deserialize it.
• Handle better some instrumentation, and generally eliminate cloning when not needed.
• Handle better some ok_or cases with an expensive None case.

[huitseeker]

Pretty dope! While I have you here fixing ok_or_else instances:

diff --git a/sui_types/src/base_types.rs b/sui_types/src/base_types.rs
index b86d5fde..314993e2 100644
--- a/sui_types/src/base_types.rs
+++ b/sui_types/src/base_types.rs
@@ -617,10 +617,7 @@ impl TryFrom<String> for ObjectID {
     type Error = ObjectIDParseError;

     fn try_from(s: String) -> Result<ObjectID, ObjectIDParseError> {
-        match Self::from_hex(s.clone()) {
-            Ok(q) => Ok(q),
-            Err(_) => Self::from_hex_literal(&s),
-        }
+        Self::from_hex(s.clone()).or_else(|_| { Self::from_hex_literal(&s) })
     }
 }

@@ -628,10 +625,7 @@ impl std::str::FromStr for ObjectID {
     type Err = ObjectIDParseError;
     // Try to match both the literal (0xABC..) and the normal (ABC)
     fn from_str(s: &str) -> Result<Self, ObjectIDParseError> {
-        match Self::from_hex(s) {
-            Ok(q) => Ok(q),
-            Err(_) => Self::from_hex_literal(s),
-        }
+        Self::from_hex(s).or_else(|_| { Self::from_hex_literal(s) })
     }
 }

😝

[huitseeker]

If you use OnceCell instead of Option, you can have just a single method digest and make it super straightforward. Please note the code comment in the below, that's where the magic happens:

struct CertifiedTransaction {
    transaction_digest: OnceCell<TransactionDigest>,
    ...
}

impl Hashable for CertifiedTransaction {
    // Note the &self here, it's not a &mut
    pub fn digest(&self) -> TransactionDigest {
        self.transaction_digest.get_or_init(|| {
            <your usual hashing implementation here>
            ...
        })
    }
}

https://docs.rs/once_cell/latest/once_cell/#

[gdanezis]

Love it love it love, and will reuse!

[gdanezis]

// Note the &self here, it's not a &mut

Yep I missed that

[gdanezis]

Many thanks @huitseeker -- now using OnceCell, and will be using it elsewhere too! Your code reviews are always a Rust masterclass.

[huitseeker]

Two problems:

1. Too many values https://github.com/huitseeker/fastnft/commit/153b50c29f6aedebca0478eea72d3d4774beeffe
2. Not enough magic https://github.com/huitseeker/fastnft/commit/7ef1f4a26138dc37098309496a9731883f9df917

[velvia]

You could just rename the original let variable as let tx_digest then here you can just do ?tx_digest. BTW I fixed this in my outstanding PR, which nobody has looked at yet. ;-)

[huitseeker]

Happy to help with codemods :D

[gdanezis]

Fixed!

[gdanezis]

Also, the only open PR that I see from you has a big "WIP:" at the start, which I took to mean you are still working on it.

[huitseeker]

Happy to help with codemods :D

[huitseeker]

Remove?

[gdanezis]

Done!

[huitseeker]

Tip top!