`at_derivation_index` needs a new error type

[tcharding]

Function at_derivation_index returns a NonDEfiniteKeyError which has 4 variants two of which are not used. One variant is confusingly called NoWildcard when the function specifically handles wildcards.

Perhaps we should have a new error type?

Found while reviewing #946

[apoelstra]

Which at_derivation_index method do you mean?

[apoelstra]

I guess you mean Descriptor::at_derivation_index, which is deprecated in favor of two other methods. It's true that each of those returns only 3 of the 4 error variants, because Wildcard and NoWildcard mean "you called one of these two functions but you should've called the other".

[tcharding]

The one in mod.rs calls through to the one in key.rs, right? And it only returns HardenedStep and Multiipath, right?

    pub fn at_derivation_index(
        self,
        index: u32,
    ) -> Result<DefiniteDescriptorKey, NonDefiniteKeyError> {
        let definite = match self {
            DescriptorPublicKey::Single(_) => self,
            DescriptorPublicKey::XPub(xpub) => {
                let derivation_path = match xpub.wildcard {
                    Wildcard::None => xpub.derivation_path,
                    Wildcard::Unhardened => xpub.derivation_path.into_child(
                        bip32::ChildNumber::from_normal_idx(index)
                            .ok()
                            .ok_or(NonDefiniteKeyError::HardenedStep)?,
                    ),
                    Wildcard::Hardened => xpub.derivation_path.into_child(
                        bip32::ChildNumber::from_hardened_idx(index)
                            .ok()
                            .ok_or(NonDefiniteKeyError::HardenedStep)?,
                    ),
                };
                DescriptorPublicKey::XPub(DescriptorXKey {
                    origin: xpub.origin,
                    xkey: xpub.xkey,
                    derivation_path,
                    wildcard: Wildcard::None,
                })
            }
            DescriptorPublicKey::MultiXPub(_) => return Err(NonDefiniteKeyError::Multipath),
        };

        DefiniteDescriptorKey::new(definite)
    }

[apoelstra]

Yes, but it's deprecated and exists as a building block for derive_at_index and into_definite, which use the other variants.

[tcharding]

This is not my wheelhouse so I'm going to close this but FTR it looks like there are two paths through the API one that expects wildcards and one that expects no wildcards. Both call through to at_derivation_index which returns the two other variants I mentioned. And all this is abstracted as a single error type.

[apoelstra]

Yes, there are two methods that conceptually do the same thing. The Wildcard error means you called the non-wildcard one but should've called the wildcard one. The NoWildcard error means the opposite. The other two variants are returned by both methods. This seems fine to me.

Meanwhile, this issue, as written, is complaining about the API of a deprecated method.