Skip to content

feat: better serialization and check tx fix - #1129

Merged
shumkov merged 1 commit into
v0.25-devfrom
feat/betterErrorSerializationAndCheckTx
May 30, 2023
Merged

shumkov merged 1 commit into
v0.25-devfrom
feat/betterErrorSerializationAndCheckTx

Conversation

@QuantumExplorer

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

Currently some errors on check_tx were causing platform to panic. Some of this was by design, however this design seemed flawed.

What was done?

All errors on check_tx return a validation result, and do not cause the system to crash.
The serialization of Consensus errors was also changed to using default bincode instead of bincode using serde.

How Has This Been Tested?

Breaking Changes

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

.map_err(|e| ResponseException::from(Error::Protocol(e)))?);

let error_data = json!({
"message": "Drive check_tx error",

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You don't need to pass the message. It will be recovered from the serialized error.

})
}
Err(error) => {
let error_data = json!({

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

We need to create a set of errors for ABCI (internal error, invalid request, and so on https://grpc.github.io/grpc/core/md_doc_statuscodes.html) eventually. Then we can translate them to gRPC error codes in DAPI. Not this PR of course.

@shumkov
shumkov merged commit 1765e8c into v0.25-dev May 30, 2023
@shumkov
shumkov deleted the feat/betterErrorSerializationAndCheckTx branch May 30, 2023 07:35
@thephez thephez added this to the v0.25.0 milestone Oct 11, 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