Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

sweep most unwraps from ethcore crate, dapps crate #2762

Merged
merged 3 commits into from
Oct 20, 2016

Conversation

rphmeier
Copy link
Contributor

Part of #1026

@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Oct 20, 2016
self.state.add_balance(&t.sender().unwrap(), &refund_value);
let sender = match t.sender() {
Ok(sender) => sender,
Err(e) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

This error is not really possible at that point (sender is validated much earlier)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, but it's not ensured at compile-time. so we could either expect (which depends on a specific usage of the function, and might get broken in the future) or just do nothing whenever the constraints of the function aren't met.

@rphmeier rphmeier added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 20, 2016
@rphmeier
Copy link
Contributor Author

Gonna sweep unwraps from a few more places in the codebase and roll it up together.

@rphmeier rphmeier changed the title sweep most unwraps from ethcore crate sweep most unwraps from ethcore crate, dapps crate Oct 20, 2016
@rphmeier rphmeier added A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Oct 20, 2016
[ci:none]
@gavofyork gavofyork added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Oct 20, 2016
@gavofyork gavofyork merged commit 96f4c10 into master Oct 20, 2016
@gavofyork gavofyork deleted the ethcore-panic-sweep branch October 20, 2016 21:41
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants