Skip to content
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

Update Proposal is not removed after expiry grace period (proposal_expiration) #79

Open
dkijania opened this issue Aug 14, 2019 · 0 comments
Assignees

Comments

@dkijania
Copy link
Contributor

UpdateState::process_proposals has bug in my opinion, which leads to problem in which proposal which was not accepted before proposal_expiration period won't be removed from proposal collection at all.

there are two conditions in mentioned method:

 if prev_date.epoch < new_date.epoch

and

else if proposal_state.proposal_date.epoch + settings.proposal_expiration
                    > new_date.epoch

if we apply proposal in the current epoch (let's say epoch 0 and our proposal_expiration is any value different than 0, then it's impossible to satisfy condition proposal_state.proposal_date.epoch + settings.proposal_expiration < new_date.epoch => 0 + 1 > 1 = false and then new_date.epoch will be increased i believe if we time proceeds, so above condition will be never met and therefore proposal will stay in collection forever).

In different scenario in which we apply proposal for epoch in the future let's say epoch 5 (while we are at epoch 0) then if proposal won't be accepted in epoch 0, then it will be immediately removed from proposals in process_proposals . (5 + 0 > 0 = true)

To fix this situation we need to reverse condition:

else if proposal_state.proposal_date.epoch + settings.proposal_expiration
                    < new_date.epoch

This will solve first scenario in epoch 2 (0+ 1 < 2 = true) and in second scenario proposal won't be removed from proposal before its time will come (5 + 0 < 0 =false )

@dkijania dkijania self-assigned this Aug 14, 2019
dkijania added a commit that referenced this issue Aug 14, 2019
dkijania added a commit that referenced this issue Aug 21, 2019
dkijania added a commit that referenced this issue Sep 9, 2019
dkijania added a commit that referenced this issue Oct 18, 2019
dkijania added a commit that referenced this issue Oct 18, 2019
#81)

* Fix for bug #79
* Introduced proposal builder. 
* Used update builder in tests
* moved e2e update test to ledger subfolder. added property and negative tests to update module
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant