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

Feature/plmc 442 delete remove from update store #180

Merged
merged 9 commits into from
Mar 11, 2024

Conversation

vstam1
Copy link
Collaborator

@vstam1 vstam1 commented Mar 6, 2024

What?

This pr deletes the remove_from_update_store function. This means that we do not remove any project transitions from the update store anymore, if these transitions are not necessary anymore. Instead we handle these unnecessary transitions by gracefully failing the second execution of these transitions. These double transition execution happen, as the project issuer can manually push a project on, or in the situation from the community round, where the round is filled during the community round duration. There were 3 cases of removal of the automatic transition:

  • do_english_auction: Auction can be started manually by the Issuer. We now fail the second do_english_auction by checking if the project is in the AuctionInitializePeriod status.
  • do_contribute: Community round was filled, so we end the funding. Automatic end of funding is now still scheduled in the update store. We fail the second funding by checking if the project status is already either of the following options:
    ProjectStatus::FundingSuccessful | ProjectStatus::FundingFailed | ProjectStatus::AwaitingProjectDecision
  • do_decide_project_outcome: In case a project is not directly failed or successful [70%, 90%], the project issuer can chose to accept or reject successful funding. In the second execution of do_project_decision we check if the project is still in the AwaitingProjectDecision phase.

Why?

To simplify the benchmarking complexity by removing the removal from project store attempts.

How?

Testing?

Fixed testing by updating the get_update_pair to take an UpdateType. As multiple round transition can be stored in update store for a project, and the order of those transitions is not always clear, we now have to explicitly specify which transition we are expecting. This is the case for the funding tests, benchmarks and the integration tests.

Screenshots (optional)

Anything Else?

@vstam1 vstam1 requested review from JuaniRios and lrazovic March 6, 2024 10:23
@vstam1 vstam1 marked this pull request as ready for review March 6, 2024 10:23
Copy link
Contributor

@JuaniRios JuaniRios left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small things and we can merge

Copy link
Member

@lrazovic lrazovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Always happy to see simplifications wherever possible. Nothing to add beyond Juan's comments.

@JuaniRios
Copy link
Contributor

I talked to @vstam1 and decided to finish the PR for him since he is working on #181.
@lrazovic can you guys take a last look and we merge?

@JuaniRios JuaniRios requested a review from lrazovic March 11, 2024 14:57
@lrazovic lrazovic requested a review from JuaniRios March 11, 2024 15:41
@JuaniRios JuaniRios merged commit 28c7bf1 into main Mar 11, 2024
@JuaniRios JuaniRios deleted the feature/plmc-442-delete-remove_from_update_store branch March 18, 2024 09:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants