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

Transaction permissions added into authority round engine as well#6469

Closed
grbIzl wants to merge 1 commit into
masterfrom
AuthorityPermissions
Closed

Transaction permissions added into authority round engine as well#6469
grbIzl wants to merge 1 commit into
masterfrom
AuthorityPermissions

Conversation

@grbIzl
Copy link
Copy Markdown
Collaborator

@grbIzl grbIzl commented Sep 5, 2017

Changes made in terms of #6441 repeated for authority round engine

@grbIzl grbIzl added A0-pleasereview 🤓 Pull request needs code review. M4-core ⛓ Core client code / Rust. labels Sep 5, 2017
@grbIzl grbIzl requested review from arkpar, keorn and rphmeier September 5, 2017 16:19
@rphmeier rphmeier added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 5, 2017
@rphmeier
Copy link
Copy Markdown
Contributor

rphmeier commented Sep 5, 2017

#6441 (comment)

the way to make that happen is not by copy-pasting this feature to other engines

at least, I feel that this logic should be

  1. not duplicated, as it leaves less room for error. this kind of per-engine duplication left us with issues adding replay protection to permissioned chains in the past.
  2. separated from the consensus layer

Glad to discuss!

@arkpar arkpar added A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Sep 20, 2017
@grbIzl
Copy link
Copy Markdown
Collaborator Author

grbIzl commented Sep 28, 2017

This fix will be made in terms other refactoring with consideration of @rphmeier comment

@grbIzl grbIzl closed this Sep 28, 2017
@rphmeier
Copy link
Copy Markdown
Contributor

@grblzl actually, it was already done in #6591

@5chdn 5chdn deleted the AuthorityPermissions branch January 3, 2018 19:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

A1-onice 🌨 Pull request is reviewed well, but should not yet be merged. M4-core ⛓ Core client code / Rust.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants