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

Replace submit_transaction endpoint with less hopeless one :D #5238

Open
dpc opened this issue May 8, 2024 · 0 comments
Open

Replace submit_transaction endpoint with less hopeless one :D #5238

dpc opened this issue May 8, 2024 · 0 comments
Assignees
Milestone

Comments

@dpc
Copy link
Contributor

dpc commented May 8, 2024

        api_endpoint! {
            SUBMIT_TRANSACTION_ENDPOINT,
            async |fedimint: &ConsensusApi, _context, transaction: SerdeTransaction| -> SerdeModuleEncoding<Result<TransactionId, TransactionError>> {
                let transaction = transaction
                    .try_into_inner(&fedimint.modules.decoder_registry())
                    .map_err(|e| ApiError::bad_request(e.to_string()))?;

                // we return an inner error if and only if the submitted transaction is
                // invalid and will be rejected if we were to submit it to consensus
                Ok((&fedimint.submit_transaction(transaction).await).into())
            }
        },

SerdeModuleEncoding<Result<TransactionId, TransactionError>>. That's terrible. Basically we encode the result using consensus encoding, making it impossible to evolve.

The top-level error is:

pub struct ApiError {
    pub code: i32,
    pub message: String,
}

so we should just impl From<TransactionError> for ApiError, and convert every case to a specific code, and throw the to_string() of the error into a message.

This will break the backward compat, so we should just make a new call for it.

Generally the way we handle errors in api is terrible, AFAICT.

BTW. If ApiError::code is the jsonrpc error code, then we should use better values, as per https://www.jsonrpc.org/specification , not http codes (see impls for ApiError):

The remainder of the space is available for application defined errors.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant