Return ProgramError from programs

[jackcmay]

Problem

It's not clear if an error came from the runtime or from a program. Distinguishing the two will help developers clarify the origin of the error. This also unifies native and BPF program return values.

Summary of Changes

• Programs return ProgramError
• MessageProcessor returns InstructionError

CI is disabled for now in this PR but both the SDK and Runtime build and are clippy free

Fixes #

[jackcmay]

@garious @jstarry Thoughts?

[garious]

I'm leaning towards no. Give me some time though.

[jstarry]

Distinguishing the two will help developers clarify the origin of the error.

Love this objective. It should be very clear to me as a program developer, which errors are caused by my program vs the runtime. I think that your PR doesn't quite make this a clean split though since native program errors (including bpf loader) are in the same namespace as user program errors.

This also unifies native and BPF program return values

I'm not sure this is a good idea because of ABI compatibility. Right now, InstructionError needs to be ABI compatible, so with this change we have coupled native program errors and user program errors to its interface.

So, if we wanted to add a new program error to our program SDK, we wouldn't be able to without making an ABI incompatible change and hard forking the network. I think it would be better to decouple user program errors so that we can be flexible and move fast with those.

[jackcmay]

Good point @jstarry about the BPF loader error overlap. That's a general problem we have with programs calling programs. BPF loader is essentially part of the runtime :-) .

We are going to have to be ABI compatible with the user program side of things soon. I was thinking that maybe a good solution for this was to make our errors extensible:
rust-lang/rfcs#757

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[garious]

@jackcmay, @jstarry, does anyone feel strongly about this PR? I don't have a strong objection. I just haven't had time to get into the app developer headspace.

[jackcmay]

@jstarry Pointed out a larger issue where we overlap program errors due to the way to treat loaders as programs and stack them. That needs to be addressed, I'll open an issue to track that.

He also advocated to not combining native program and user program errors. I'm not so concerned about that since we are going to have to be ABI compatible soon and we can maybe use extensible enums.

Pretty much this PR has been put on hold due to other priorities but the needs it attempts to address still exist. I think filing a few issues and closing this, for now, is probably the best course of action.

[garious]

Sgtm

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

[stale]

This stale pull request has been automatically closed. Thank you for your contributions.