8000 only last leader in epoch can pin by ThomasArts · Pull Request #4451 · aeternity/aeternity · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

only last leader in epoch can pin #4451

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 6 commits into from
Nov 12, 2024
Merged

only last leader in epoch can pin #4451

merged 6 commits into from
Nov 12, 2024

Conversation

ThomasArts
Copy link
Contributor

Fixes Issue #4448 in a particular way.

Instead of carrying around who the last leader is in the epochs (which resulted in rather complex code and unsatisfactory solution for first two epochs), decided to keep the contract simple and enforce pin call to be added to the last micro block. In that way, it is easy to check who the leader is.

Up to debate when to clear the PIN. If done in step_eoe then inspecting the last key block will not show it. Hence one needs to look before that. If done in step then extra check needed that this is first block of epoch or reset to NONE in each call (since one cannot set it anyway).

This PR is supported by the Aeternity foundation

@ThomasArts ThomasArts force-pushed the GH4448-last-leader-known branch from 97f0ad2 to 7957c70 Compare October 23, 2024 13:12
@ThomasArts ThomasArts mentioned this pull request Oct 24, 2024
Closed
@ThomasArts
Copy link
Contributor Author

After some discussion with @hanssv we realized that this won't work, because in the micro block before a key block, the leader is not updated... Leaders are updated in step_eoe when the key block is produced.
This is a remainder of having micro blocks after key blocks. However, it makes sense to actually fix that, because other contracts may also want to do things relative to current leader.

Opened issue #4453 to get a solution on that.
Alternative is the the contract logic is changed and that it from start of epoch stores in its state the last leader. That is less elegant and does not solve any future issue with other than the last block in an epoch.

@ThomasArts ThomasArts added status/wip Issues or PRs which are not ready for review or further actions area/hyperchains labels Oct 24, 2024
@ThomasArts ThomasArts force-pushed the GH4448-last-leader-known branch 6 times, most recently from de71e9b to 28f4a7f Compare November 4, 2024 08:56
@ThomasArts ThomasArts force-pushed the GH4448-last-leader-known branch from 28f4a7f to 247533a Compare November 7, 2024 08:10
@hanssv hanssv force-pushed the GH4448-last-leader-known branch from 247533a to ff62e72 Compare November 11, 2024 15:19
@hanssv hanssv removed the status/wip Issues or PRs which are not ready for review or further actions label Nov 12, 2024
ThomasArts
ThomasArts commented Nov 12, 2024
View reviewed changes
Copy link
Contributor Author
@ThomasArts ThomasArts left a comment

Choose a reason for hiding this comment

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

Looked at it, but as initiator I cannot approve

klercker
klercker reviewed Nov 12, 2024
View reviewed changes
Copy link
Contributor
@klercker klercker left a comment

Choose a reason for hiding this comment

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

Sure, I can live with this. Once the pinning agent is up, these "artificial test will likely be removed, or at least changed quite a bit.

@hanssv hanssv merged commit c6c7b93 into master Nov 12, 2024
40 checks passed
@hanssv hanssv deleted the GH4448-last-leader-known branch November 12, 2024 09:56
@dincho dincho added the kind/feature Issues or PRs related to a new feature label Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@happi happi happi approved these changes

@klercker klercker klercker approved these changes

Assignees
No one assigned
Labels
area/hyperchains kind/feature Issues or PRs related to a new feature
Projects
Periodically Syncing Hyperchains
Status: Done
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ThomasArts @happi @klercker @hanssv @dincho
0