Miscellaneous code improvements for Orchestrator #364
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leviathanbeak
commented
Oct 30, 2021
| } else if req_delegate_orchestrator_address != delegate_orchestrator_address { | ||
| error!("Your Delegate Orchestrator address is incorrect!"); | ||
| error!( | ||
| "You provided {} Correct Value {}", | ||
| delegate_eth_address, req_delegate_eth_address |
There was a problem hiding this comment.
Also I think this is a bug? The addresses in question are those from orchestrator and not eth? and in error! eth addresses are provided?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Solves #360
I have made the improvements mentioned in the issue, having said that, handling and propagating errors is still not completed, while it could be argued it could be a separate ticket in itself, since I did a lot of work in this ticket, being provided some guidelines I could implement better error propagation.
What the audit recommended is that we handle errors at the top not somewhere randomly in lib functions, and exiting the app without any clear hints to the caller of the functions. I went and created another Error Enum by using thiserror lib in our lib code, and also used anyhow lib in main.rs.
I have not cleaned all exit(1) from other functions, cuz I wanted some feedback on the current improvements, especially concerning error handling, and naming of ValidityCheckError - not sure what name would be best, since I would need much more context on all of it :)