-
Notifications
You must be signed in to change notification settings - Fork 903
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
(WIP) Auditor.checkAllLedgers should use Auditor's instance variables #1588
base: master
Are you sure you want to change the base?
Conversation
- currently checkAllLedgers method creates instances of ZK, BK and BKAdmin everytime this method is called. Instead it should use Auditors instance variables.
retest this please |
few replication tests are failing. Will check that. |
the change here will introduce deadlock so this PR needs to be revisited. See comment: |
@reddycharan this patch got enough consensus to be committed. cc @sijie |
Oh I missed @sijie 's comment. we should revisit this patch or close it |
@reddycharan this patch was already approved by @sijie do you mind rebasing it to current master ? |
@reddycharan can you please rebase to master and push? This is a useful patch to have. |
@eolivelli @sijie I went through the current implementation of |
Descriptions of the changes in this PR:
everytime this method is called. Instead it should use Auditors instance
variables.