Skip to content

Conversation

@lucad70
Copy link
Contributor

@lucad70 lucad70 commented Jun 2, 2025

What is the purpose of this pull request?

  • Bug fix
  • Documentation update
  • New feature
  • Test
  • Other: Fix linting

Which crates are being modified?

  • floresta-chain
  • floresta-cli
  • floresta-common
  • floresta-compact-filters
  • floresta-electrum
  • floresta-watch-only
  • floresta-wire
  • floresta
  • florestad
  • Other:

Description

The CI on #497 was failing after nightly update. After carefully checking where the structs KeyVersion and DefaultResolver were (or could) be used, I opted for removing them. I believe the current code using data variable is self explanatory. The DefaultResolver however, I could not see where it should be used.

Notes to the reviewers

One method where the KeyVersion could be used was on from_slip132_str but I could not find the specific usage for DefaultResolver, so I would appreciate insights into this.

impl FromSlip132 for Xpub {
    fn from_slip132_str(s: &str) -> Result<Self, Error> {
        let mut data = base58::decode_check(s)?;

        let mut prefix = [0u8; 4];
        prefix.copy_from_slice(&data[0..4]);
        let slice = match prefix {
            VERSION_MAGIC_XPUB
            | VERSION_MAGIC_YPUB
            | VERSION_MAGIC_ZPUB
            | VERSION_MAGIC_YPUB_MULTISIG
            | VERSION_MAGIC_ZPUB_MULTISIG => VERSION_MAGIC_XPUB,

            VERSION_MAGIC_TPUB
            | VERSION_MAGIC_UPUB
            | VERSION_MAGIC_VPUB
            | VERSION_MAGIC_UPUB_MULTISIG
            | VERSION_MAGIC_VPUB_MULTISIG => VERSION_MAGIC_TPUB,

            _ => return Err(Error::UnknownSlip32Prefix),
        };
        data[0..4].copy_from_slice(&slice);

        let xpub = Xpub::decode(&data)?;

        Ok(xpub)
    }
}

impl FromSlip132 for Xpriv {
    fn from_slip132_str(s: &str) -> Result<Self, Error> {
        let mut data = base58::decode_check(s)?;

        let mut prefix = [0u8; 4];
        prefix.copy_from_slice(&data[0..4]);
        let slice = match prefix {
            VERSION_MAGIC_XPRV
            | VERSION_MAGIC_YPRV
            | VERSION_MAGIC_ZPRV
            | VERSION_MAGIC_YPRV_MULTISIG
            | VERSION_MAGIC_ZPRV_MULTISIG => VERSION_MAGIC_XPRV,

            VERSION_MAGIC_TPRV
            | VERSION_MAGIC_UPRV
            | VERSION_MAGIC_VPRV
            | VERSION_MAGIC_UPRV_MULTISIG
            | VERSION_MAGIC_VPRV_MULTISIG => VERSION_MAGIC_TPRV,

            _ => return Err(Error::UnknownSlip32Prefix),
        };
        data[0..4].copy_from_slice(&slice);

        let xprv = Xpriv::decode(&data)?;

        Ok(xprv)
    }
}

Author Checklist

  • I've followed the contribution guidelines
  • I've verified one of the following:
    • Ran just pcc (recommended but slower)
    • Ran just lint-features '-- -D warnings' && cargo test --release
    • Confirmed CI passed on my fork
  • I've linked any related issue(s) in the sections above

Finally, you are encouraged to sign all your commits (it proves authorship and guards against tampering—see How (and why) to sign Git commits and GitHub's guide to signing commits).

@Davidson-Souza Davidson-Souza added CI A change or issue related to CI lint This issue or PR relates to code style and linting labels Jun 2, 2025
@Davidson-Souza
Copy link
Member

One method where the KeyVersion could be used was on from_slip132_str but I could not find the specific usage for DefaultResolver, so I would appreciate insights into this.

It may not be used anywhere. This code was originally copied from another lib, with some functionalities stripped out as we didn't need them. We can remove everything we don't need from this file without any problem.

Copy link
Member

@Davidson-Souza Davidson-Souza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK 424c20a

@jaoleal
Copy link
Collaborator

jaoleal commented Jun 2, 2025

ACK 424c20a

@Davidson-Souza Davidson-Souza merged commit ecc8650 into vinteumorg:master Jun 2, 2025
10 checks passed
@Davidson-Souza Davidson-Souza mentioned this pull request Jun 2, 2025
15 tasks
@lucad70 lucad70 deleted the fix-linting branch July 10, 2025 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI A change or issue related to CI lint This issue or PR relates to code style and linting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants