Improve error message

[Eunovo]

This PR improves the error message returned for blockchain.transaction.get when the bitcoin daemon cannot find the transaction. See #936

[antonilol]

might be worth to define all error codes (in a new enum) to allow for easy future mapping of them to a different or more verbose message (like this pr)

[Eunovo]

might be worth to define all error codes (in a new enum) to allow for easy future mapping of them to a different or more verbose message (like this pr)

I have added all the rpc error codes defined in bitcoin's protocol.h file

[antonilol]

The enum members don't need the i32 fields, they will only every be set to one value define them like this: RpcInvalidRequest = -32600,

and the enum definition and parse_code function can be macro generated, to not have 2 places where these codes are mapped to the enum member (but 1) to avoid inconsistencies/desyncs

[Eunovo]

The enum members don't need the i32 fields, they will only every be set to one value define them like this: RpcInvalidRequest = -32600,

and the enum definition and parse_code function can be macro generated, to not have 2 places where these codes are mapped to the enum member (but 1) to avoid inconsistencies/desyncs

@antonilol All done

[antonilol]

the documentation of the error codes can also be copied from the header file of bitcoind source i meant something different with macro generated (macro_rules! ...), but maybe thats just me that i dont like introducing new dependencies. going from an enum variant to a number is as easy as the as keyword (as i16 in this case), proc macros also add (sometimes significant) extra compile time dont get me wrong, this works like i intended but adds a dependency

[Eunovo]

the documentation of the error codes can also be copied from the header file of bitcoind source

Are you referring to the comments in the protocol.h file?

i meant something different with macro generated (macro_rules! ...), but maybe thats just me that i dont like introducing new dependencies. going from an enum variant to a number is as easy as the as keyword (as i16 in this case), proc macros also add (sometimes significant) extra compile time dont get me wrong, this works like i intended but adds a dependency

I'm not sure I understand what you mean here. Can I generate a from_i16 implementation on the enum with just declarative macros (macro_rules!)? Even if I remove the num_traits dependency, it looks like I'll still have to create a proc_macro and that will require that I add the syn crate to process the inputs to the macro.

[antonilol]

Are you referring to the comments in the protocol.h file?

yes

Can I generate a from_i16 implementation on the enum with just declarative macros (macro_rules!)?

yes

with something along the lines of

macro_rules! define_error_codes {
    ($($name:ident = $value:expr,)*) => {
        /// some doc comments
        // some derives
        #[derive(Debug, Clone, Copy)]
        #[repr(i16)]
        pub enum CoreRPCErrorCode {
            $(
                $name = $value,
            )*
        }

        impl CoreRPCErrorCode {
            pub fn from_error_code(code: i16) -> Option<Self> {
                Some(match code {
                    $(
                        $value => Self::$name,
                    )*
                    _ => return None,
                })
            }
        }
    };
}

define_error_codes! {
    RpcInvalidRequest = -32600,
    RpcMethodNotFound = -32601,
    RpcInvalidParams = -32602,
    RpcInternalError = -32603,
    RpcParseError = -32700,

    ... etc
}

impl CoreRPCErrorCode {
    pub fn to_error_code(self) -> i16 {
        self as i16
    }
}

it can also be made more flexible but i dont think that is needed (at least yet), but you could also take the name and visibility of the enum as metavariables.

[Eunovo]

@antonilol Thanks for your help with the macro_rules! definition. I've changed the implementation to use that and I also added doc comments. Please take a look and let me know if there's anything I need to change.

[antonilol]

@antonilol Thanks for your help with the macro_rules! definition. I've changed the implementation to use that and I also added doc comments. Please take a look and let me know if there's anything I need to change.

this is great!

(test fails on a type mismatch, change the input type of from_error_code from i16 to i32 and should be fine)

[Eunovo]

@antonilol I had to remove the to_error_code fn because it was never used and it caused the tests to fail

[Eunovo]

@romanz Is there anything else I need to address before this can be merged?