Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't load single-sig cache #199

Closed
pavel-kokolemin opened this issue Oct 19, 2023 · 2 comments
Closed

Can't load single-sig cache #199

pavel-kokolemin opened this issue Oct 19, 2023 · 2 comments
Assignees

Comments

@pavel-kokolemin
Copy link

When I restart my single-sig testnet liquid wallet, loading the cache fails with an error here:

impl RawCache {
    fn try_new<P: AsRef<Path>>(path: P, cipher: &Aes256GcmSiv) -> Result<Self, Error> {
        let decrypted = load_decrypt(Kind::Cache, path, cipher)?;
        let store = ciborium::from_reader(&decrypted[..])?; // Fails with the "invalid type: bytes, expected bytes" error
        Ok(store)
    }

As a result, loading the single-sig wallet after a restart is slow because it downloads everything from scratch.

Looks like the problem is caused by the derived BETransaction serde implementation:

#[derive(Debug, Clone, Serialize, Deserialize, Hash)]
pub enum BETransaction {
    Bitcoin(bitcoin::Transaction),
    Elements(elements::Transaction),
}

I tried this as a test and it worked fine (balance is available almost instantly after restart):

impl serde::Serialize for BETransaction {
    fn serialize<S: serde::Serializer>(&self, serializer: S) -> Result<S::Ok, S::Error> {
        match self {
            BETransaction::Bitcoin(_) => todo!(),
            BETransaction::Elements(tx) => {
                serializer.serialize_str(&elements::encode::serialize_hex(tx))
            }
        }
    }
}

impl<'de> serde::Deserialize<'de> for BETransaction {
    fn deserialize<D: serde::Deserializer<'de>>(deserializer: D) -> Result<Self, D::Error> {
        let hex = String::deserialize(deserializer)?;
        let vec = Vec::<u8>::from_hex(&hex).map_err(|e| serde::de::Error::custom(e))?;
        let value = elements::encode::deserialize(&vec).map_err(|e| serde::de::Error::custom(e))?;
        Ok(BETransaction::Elements(value))
    }
}
@RCasatta RCasatta self-assigned this Oct 19, 2023
@RCasatta
Copy link
Contributor

I confirm the issue, thanks for reporting and identifying the cause.

The bug has been introduced when switching from serde_cbor to ciborium in cba5205, so from gdk 0.68.0 .

This is happening also in upstream rust-elements ElementsProject/rust-elements@45bd2bf and it's related to how the RangeProof is deserialized.

The custom Deserialize in RangeProof seems fine though, and the fact the serde_cbor works supports this hypothesis, so I think the root cause is even more upstream enarx/ciborium#96

If the issue is not in upstream lib or is not fixed in a timely manner, we will implement something along the lines you suggested, maybe using the consensus serialization instead of hex serialization to save some space (if that deserialize correctly)

@RCasatta
Copy link
Contributor

https://github.com/Blockstream/gdk/releases/tag/release_0.68.4 fixes this.

In the end we reverted back to using serde_cbor. More details in 3d3aa33

I am leaving this open for a couple of days if you want to give feedback

@RCasatta RCasatta reopened this Oct 26, 2023
@RCasatta RCasatta closed this as completed Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants