-
Notifications
You must be signed in to change notification settings - Fork 0
dbft: add requestTxs for dbft #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
roman-khimov
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
|
This PR is valid for CNs requesting missing transactions from peers under high transactions load. 4cn_cn1.log
4cn_cn2.log
4cn_cn3.log
4cn_cn4.log
|
|
CN requested for missing txs, but got Details
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
@chenquanyu, could you please squash the commits into a single commit (may be done with git rebase -i HEAD~2) and name this commit something like "dbft: implement WithRequestTx callback" and add Close #59. message to the commit body? Like described in https://github.com/nspcc-dev/.github/blob/master/git.md#commits.
Minor comments are left to be fixed, after that ready to be merged. We're going to include this PR into NeoX T2 version.
@nicolegys, I think this is done because we request the same transactions from multiple different peers. First peer responds, the node saves transaction, then second peer responds and the node already have this transaction got from the first peer. We have #97 to improve this behaviour. |
b506b8f to
4ebff76
Compare
4ebff76 to
78d480e
Compare
68085e0 to
7e713d2
Compare
AnnaShaleva
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nicolegys, @roman-khimov agree to merge?


Close #59 Integrate transaction request mechanism.