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

Cleanup some code in Aura #11466

Merged
merged 2 commits into from
Feb 10, 2020
Merged

Conversation

dvdplm
Copy link
Collaborator

@dvdplm dvdplm commented Feb 7, 2020

Nothing really interesting here, renames or removes some methods. Adds some docs and extends a test a bit to clarify the behaviour of the code.

Nothing really interesting here, renames or removes some methods. Adds some docs and extends a test a bit to clarify the behaviour of the code.
@dvdplm dvdplm self-assigned this Feb 7, 2020
@dvdplm dvdplm added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). labels Feb 7, 2020
Copy link
Collaborator

@ordian ordian left a comment

Choose a reason for hiding this comment

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

Looks good modulo one question.


if let Ok(c) = self.upgrade_client_or("could not broadcast empty step message") {
self.store_empty_step(empty_step);
c.broadcast_consensus_message(empty_step_full_rlp(&signature, &empty_step_rlp));
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't know if it makes any difference, but previously a message was broadcasted before storing it and irregardless of whether client upgrade ref was successfull

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Indeed, I think that behaviour is slightly suspicious. I have no proof that it was wrong, but I think the new behaviour is safer.

Co-Authored-By: Niklas Adolfsson <niklasadolfsson1@gmail.com>
@ordian ordian merged commit 2c4b51c into master Feb 10, 2020
@ordian ordian deleted the dp/chore/cleanup-and-docs-for-aura-empty-steps branch February 10, 2020 13:26
dvdplm added a commit that referenced this pull request Feb 10, 2020
* master:
  Use parity-crypto updated to use upstream rust-secp256k1 (#11406)
  Cleanup some code in Aura (#11466)
  upgrade some of the dependencies (#11467)
  Some more release track changes to README.md (#11465)
  Update simple one-line installer due to switching to a single stable release track (#11463)
  update Dockerfile (#11461)
  Implement EIP-2124 (#11456)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-pleasereview 🤓 Pull request needs code review. A2-insubstantial 👶 Pull request requires no code review (e.g., a sub-repository hash update). M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants