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

rusk-wallet: Moonlight addresses are not always accepted as valid BLS keys #2580

Open
HDauven opened this issue Oct 3, 2024 · 2 comments · May be fixed by #2594
Open

rusk-wallet: Moonlight addresses are not always accepted as valid BLS keys #2580

HDauven opened this issue Oct 3, 2024 · 2 comments · May be fixed by #2594
Assignees
Labels
fix:bug Something isn't working module:rusk-wallet Issues related to rusk wallet

Comments

@HDauven
Copy link
Member

HDauven commented Oct 3, 2024

Describe the bug
A number of Moonlight addresses are not accepted as valid BLS keys when staking, unstaking or transferring funds.

To Reproduce
Try to transfer funds to any of the following addresses:

yqS28Gu35xGNoDgo4eFpkUzQCWvMXuaZjKigmSY4xKNc5DSYJzH5ovu82WxYV8kt1q3rhKo9cXncxWjqohHVcVdGBpd9MztcLjRUsJvNDchCCiRFT7zFM8K4C4JEjTPKyk6

ord8D9WF21NPJmaDc3hN9wLnqRTcNsLwFVwUNZW9JuYmQhpbotxrBKXD25M4w76GXbTRS8nmr8y9jmriaw5tp2yhXLBHL47DK6S2eug2P3K5fYgCcJHEU2suaM8SaCTaek5

Expected behaviour
These addresses were generated with mnemonics accepted by our wallet.
Either the:

  • BIP9 implementation being used/enforced is wrong
  • validation of BLS keys is wrong
  • the BLS keys generation is wrong

Logs/Screenshot
image

@HDauven HDauven added fix:bug Something isn't working module:rusk-wallet Issues related to rusk wallet labels Oct 3, 2024
@moCello moCello self-assigned this Oct 3, 2024
@tminus92
Copy link

tminus92 commented Oct 3, 2024

Regarding the failed moonlight transfers, we tracked the issue to the parsing of the BLS key from the command line

The code doing the string to Address parsing

impl FromStr for Address {
    type Err = Error;

    fn from_str(s: &str) -> Result<Self, Self::Err> {
        let try_phoenix = Self::try_from_str_phoenix(s);
        let try_bls = Self::try_from_str_bls(s);

        if let Ok(addr) = try_phoenix {
            Ok(addr)
        } else {
            try_bls
        }
    }
}

pub fn try_from_str_phoenix(s: &str) -> Result<Self, Error> {
    let bytes = bs58::decode(s).into_vec()?;

    let pk = PhoenixPublicKey::from_reader(&mut &bytes[..])
        .map_err(|_| Error::BadAddress)?;

    let bytes2 = pk.to_bytes();
        
    let addr = Self::Phoenix {
        index: None,
        addr: pk,
    };

    Ok(addr)
}

The from_reader call goes into each respective key default trait implementation therefore I am not gonna add it. The result of the try_from_str_phoenix function is a phoenix key constructed from the first 64 bytes out of the original 96 bytes from the BLS key. The expected result should be a BadLength error

@tminus92
Copy link

tminus92 commented Oct 3, 2024

Regarding the failed moonlight unstake, we tracked the issue to the wallet trying to unstake to the phoenix key instead of the BLS one

@moCello moCello linked a pull request Oct 3, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix:bug Something isn't working module:rusk-wallet Issues related to rusk wallet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants