bump spl lib errors

[buffalojoec]

This PR addresses the problems presented by #5042 , however I would like
to discuss a deeper issue that may cause a wider range of problems for
downstream consumers of our SPL libraries.

Problem

Our libraries have various methods that are designed to return ProgramError
in order to make using them within on-chain programs seamless and concise.
However, the concept of mapping a library's error directly to a program error
is actually a flawed design.

The ProgramError::Custom(u32) variant is specifically designed for an
on-chain program to implement any number of errors that can be mapped to a
u32 error code from 0 to u32::MAX. These custom codes are intended for
errors that are specific to that program and have no relevance outside the
scope of that program's crate.

Thus, we cannot force errors from shared libraries to map to
ProgramError::Custom(u32) using pre-defined u32 values. This will only
cause more issues related to #5042 and things will get further
complicated with libraries that are built on other libraries, and so on.

Solution

Decouple library errors from ProgramError, and instead provide tooling to
easily map library errors to ProgramError within on-chain programs.

Temporary Solution

Simply bump the library errors to be very large u32 values with
deterministic offsets.

Also create an issue describing the above problem.

[buffalojoec]

CI failed on my first commit (78559d9) that the error codes are now being properly recognized as spawning from their respective source libraries.

The following commit (2dabcd5) pushes a fix to these error checks.

[buffalojoec]

I'm thinking the longer term solution might be to implement something like this across our libraries:

use {
    num_derive::FromPrimitive,
    solana_program::{
        decode_error::DecodeError,
        msg,
        program_error::{PrintProgramError, ProgramError},
    },
    thiserror::Error,
};

// We introduce this trait and enum combination so our libraries can define
// functions that may return their own defined errors or a known `ProgramError`

pub trait SplError<E>:
    'static + std::error::Error + DecodeError<E> + num_traits::FromPrimitive
{
}

pub enum LibraryError<E>
where
    E: 'static + std::error::Error + DecodeError<E> + num_traits::FromPrimitive,
{
    LibraryError(E),
    ProgramError(ProgramError),
}

impl<E> From<E> for LibraryError<E>
where
    E: SplError<E>,
{
    fn from(e: E) -> Self {
        Self::LibraryError(e)
    }
}

impl<E> From<ProgramError> for LibraryError<E>
where
    E: 'static + std::error::Error + DecodeError<E> + num_traits::FromPrimitive,
{
    fn from(e: ProgramError) -> Self {
        Self::ProgramError(e)
    }
}

// Then we define our library errors _without_ any `program_error` traits
// but with `DecodeError` and `SplError`

#[derive(Clone, Debug, Eq, Error, FromPrimitive, PartialEq)]
pub enum TlvError {
    #[error("...")]
    SomeTlvError,
    #[error("...")]
    AnotherTlvError,
}

impl<T> DecodeError<T> for TlvError {
    fn type_of() -> &'static str {
        "TlvError"
    }
}

impl SplError<TlvError> for TlvError {}

// Now we can define our library functions to return `LibraryError<TlvError>`

pub fn library_function() -> Result<(), LibraryError<TlvError>> {
    Err(TlvError::SomeTlvError.into())
}

// Now let's examine an on-chain program's errors

#[derive(Clone, Debug, Eq, Error, FromPrimitive, PartialEq)]
pub enum MyProgramError {
    #[error("...")]
    SomeError,
    #[error("...")]
    AnotherError,
}

// Implement all three traits as normal

impl From<MyProgramError> for ProgramError {
    fn from(e: MyProgramError) -> Self {
        ProgramError::Custom(e as u32)
    }
}

impl<T> DecodeError<T> for MyProgramError {
    fn type_of() -> &'static str {
        "MyProgramError"
    }
}

impl PrintProgramError for MyProgramError {
    fn print<E>(&self)
    where
        E: 'static + std::error::Error + DecodeError<E> + num_traits::FromPrimitive,
    {
        msg!("Error: {}");
    }
}

// Now we also add this bad boy, which will describe how to convert the library
// errors into `ProgramError`s _only_ within our crate.

impl From<LibraryError<TlvError>> for ProgramError {
    fn from(e: LibraryError<TlvError>) -> Self {
        match e {
            LibraryError::LibraryError(e) => ProgramError::Custom(
                (e as u32).saturating_add(1000) // Offset
            ),
            LibraryError::ProgramError(e) => e,
        }
    }
}

// Now we can define our program's functions to return `ProgramError`

pub fn program_function() -> Result<(), ProgramError> {
    Err(MyProgramError::SomeError.into())
}

// And we can use our library functions within our program

pub fn program_function_using_library() -> Result<(), ProgramError> {
    library_function()?;
    Ok(())
}

However, this spec would need some work since we'd run into trait collisions if this program's crate was imported by another program.

Perhaps something along the lines of feature flags could help?

#[cfg(feature("entrypoint")]
impl From<LibraryError<TlvError>> for ProgramError {
    fn from(e: LibraryError<TlvError>) -> Self {
        match e {
            LibraryError::LibraryError(e) => ProgramError::Custom(
                (e as u32).saturating_add(1000) // Offset
            ),
            LibraryError::ProgramError(e) => e,
        }
    }
}

[buffalojoec]

HOWEVER... Simply adding >10_000 to the error codes that stem from libraries should do the trick for the foreseeable future. I just figured this might be worth a discussion.

Let me know any thoughts @joncinque

[joncinque]

Why don't we do the same as with instructions -- hash the name with a namespace, and take 4 bytes from the middle as a discriminant?

[buffalojoec]

Why don't we do the same as with instructions -- hash the name with a namespace, and take 4 bytes from the middle as a discriminant?

Yeah that's not bad. Maybe use the middle 4 bytes as a multiplier? Say, by 1,000?
That way, we are clear from the low-value numbers (Anchor goes up to 5,000) and give tons of range between discriminators that might be off by one byte.

Also curious why middle and not first 4?

hash("spl_error:tlv_error")[4..8]
// 0_0002_949_000

[joncinque]

Yeah that's not bad. Maybe use the middle 4 bytes as a multiplier? Say, by 1,000?

No no, just the middle 4 bytes as a little-endian u32. So that way every error is unique!

Also curious why middle and not first 4?

If we can believe https://discord.com/channels/428295358100013066/967028962746327060/1144342017200107610, there's higher entropy there? Also, people typically grind out the first few bytes, as I did for those metadata instruction discriminators. But then again, once they know where it's taken from, they could also grind those bytes, so it might not make a huge difference.

[buffalojoec]

No no, just the middle 4 bytes as a little-endian u32. So that way every error is unique!

OK gotcha 🫡

If we can believe https://discord.com/channels/428295358100013066/967028962746327060/1144342017200107610, there's higher entropy there?

I mean, I buy it (?). I guess I'll see how weird it looks.

[buffalojoec]

Closing for #5169 !