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

bugfix: mobilecoind should return a grpc NOT_FOUND error code when ledger data is not found #3787

Merged
merged 4 commits into from
Dec 11, 2023

Conversation

eranrund
Copy link
Contributor

Previously, mobilecoind returned a GRPC INVALID_ARGUMENT error when certain calls were made to the ledger database and the data requested wasn't found.
NOT_FOUND is a more correct error code in such cases, and is required for correct behavior of the new remote-mobilecoind fog BlockProvider (see https://github.com/mobilecoinfoundation/mobilecoin/blob/feature/bugfix-ledger-notfound-err-code/fog/block_provider/src/error.rs#L38-L39 and https://github.com/mobilecoinfoundation/mobilecoin/blob/feature/bugfix-ledger-notfound-err-code/fog/ledger/server/src/merkle_proof_service.rs#L156)

To verify this results in the expected behavior I hacked together a client that calls get_outputs on the fog merkleproof service. Prior to this change, I got this result:

Rpc(RpcFailure(RpcStatus { code: 13-INTERNAL, message: "Database Error: GRPC: RpcFailure: 3-INVALID_ARGUMENT get_tx_out_by_index: Record not found", details: [], debug_error_string: "UNKNOWN:Error received from peer  {created_time:\"2023-12-11T11:44:49.535884-08:00\", grpc_status:13, grpc_message:\"Database Error: GRPC: RpcFailure: 3-INVALID_ARGUMENT get_tx_out_by_index: Record not found\"}" }))

And after the change:

GetOutputsResponse { results: [OutputResult { index: 505555455555555555, result_code: 1, output: TxOut { masked_amount: None, target_key: CompressedRistrettoPublic(0000000000000000000000000000000000000000000000000000000000000000), public_key: CompressedRistrettoPublic(0000000000000000000000000000000000000000000000000000000000000000), e_fog_hint: EncryptedFogHint(000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000), e_memo: None }, proof: TxOutMembershipProof { index: 0, highest_index: 0, elements: [] } }], num_blocks: 6030, global_txo_count: 138904, latest_block_version: 3, max_block_version: 3 }

Note that result_code 1 is NotFound (https://github.com/mobilecoinfoundation/mobilecoin/blob/feature/bugfix-ledger-notfound-err-code/fog/api/proto/ledger.proto#L173)

@eranrund eranrund changed the base branch from main to release/v5.2 December 11, 2023 20:31
Copy link
Collaborator

@nick-mobilecoin nick-mobilecoin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The parse_transfer_code_impl() seems like the mapping might be in the wrong place, not sure if it's necessary for the immediate need.

mobilecoind/src/service.rs Outdated Show resolved Hide resolved
@jgreat jgreat merged commit a6ae003 into release/v5.2 Dec 11, 2023
102 of 122 checks passed
@jgreat jgreat deleted the feature/bugfix-ledger-notfound-err-code branch December 11, 2023 21:55
@nick-mobilecoin nick-mobilecoin mentioned this pull request Dec 14, 2023
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

Successfully merging this pull request may close these issues.

3 participants