Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Why is mem_round flushed twice while finishing a round #216

Closed
sqfasd opened this issue Jun 22, 2016 · 8 comments
Closed

Why is mem_round flushed twice while finishing a round #216

sqfasd opened this issue Jun 22, 2016 · 8 comments

Comments

@sqfasd
Copy link

sqfasd commented Jun 22, 2016

https://github.com/LiskHQ/lisk/blob/development/modules/round.js#L136
https://github.com/LiskHQ/lisk/blob/development/modules/round.js#L137

return this.updateVotes()
            .then(this.updateMissedBlocks)
            .then(this.flushRound)
            .then(this.applyRound)
            .then(this.updateVotes)
            .then(this.flushRound)
            .then(function () {
                private.ticking = false;
                return t;
            });

I think the later updateVotes and flushRound can be removed.

@karmacoma
Copy link
Contributor

The current procedure is as follows:

  • Update votes (from account balances)
  • Update delegates which have missed blocks
  • Flush the round
  • Apply fees, rewards to eligible delegates
  • Update votes (from account balances) - i.e. changed by fees, rewards
  • Flush the round

Which could be changed to something like:

  • Update delegates which have missed blocks
  • Apply fees, rewards to eligible delegates
  • Update votes (from account balances)
  • Flush the round

Requires further implementation and testing.

@karmacoma karmacoma added this to the Version 0.4.0 milestone Aug 2, 2016
@karmacoma karmacoma modified the milestone: Community Forging Aug 17, 2016
@karmacoma karmacoma removed their assignment Dec 9, 2016
@karmacoma karmacoma changed the title why mem_round was flushed twice while finishing a round Why is mem_round flushed twice while finishing a round Apr 21, 2017
@karmacoma karmacoma added this to the Version 1.0.0 milestone Apr 21, 2017
@karmacoma
Copy link
Contributor

Should be resolved after: #543 is closed.

@karmacoma
Copy link
Contributor

Closed by: #597

@4miners
Copy link
Contributor

4miners commented Jan 8, 2018

Need recheck because #597 is reverted.

@4miners 4miners reopened this Jan 8, 2018
@diego-G diego-G self-assigned this Feb 19, 2018
@diego-G
Copy link

diego-G commented Feb 19, 2018

It should be reviewed after #1303

@MaciejBaj
Copy link
Contributor

Re-opened as the fix was inverted as part of #2420.

@MaciejBaj MaciejBaj reopened this Oct 8, 2018
@MaciejBaj MaciejBaj removed this from the Version 1.1.0 milestone Oct 8, 2018
@MaciejBaj
Copy link
Contributor

Can only be solved after #2423 is done, which is waiting for the next hard fork as breaking. Suspended for now.

@shuse2
Copy link
Collaborator

shuse2 commented Jul 29, 2019

This will be addressed with part of the BFT implementation #3555

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

7 participants