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

[HOTFIX] fix leader crosslink issue to not include old cross link in the propo… #4629

Merged
merged 11 commits into from
Feb 13, 2024

Conversation

GheisMohammadi
Copy link
Contributor

@GheisMohammadi GheisMohammadi commented Feb 12, 2024

Issue

The leader is adding pending crosslinks without verifying them. This could break the consensus if other validators don't have the old shard state needed to verify crosslinks. This PR addresses that issue by ignoring very old crosslinks and not adding them to proposing blocks.

The old pending crosslink will also be deleted. Deletion will happen when the node start, and anytime invalid crosslink are seen during the leader proposal process

@sophoah
Copy link
Contributor

sophoah commented Feb 12, 2024

Just to add on, afaik, crosslink are today only used to calculate the reward to be distributed every 64 blocks, hence discarding those old pending crosslink should be safe and shouldn't affect the protocol

@ONECasey ONECasey added the high priority high priority issue with customer impact label Feb 12, 2024
@diego1q2w diego1q2w merged commit 313daea into main Feb 13, 2024
2 of 4 checks passed
@diego1q2w diego1q2w deleted the patch/leader_crosslink branch February 13, 2024 08:21
ONECasey added a commit that referenced this pull request Feb 21, 2024
* fix allowed txs to be able to handle multiple txs for same from address (#4624)

* [HOTFIX] fix leader crosslink issue to not include old cross link in the propo… (#4629)

* fix leader crosslink issue to not include old cross link in the proposing block

* set higher epoch threshold for pending crosslinks to be added to proposing block

* delete old pending cross links

* delete when proposing

* delete when proposing

* delete when proposing

* delete when proposing

* minor logic change for the log

* minor logic change for the log

* minor logic change for the log

* minor logic change for the log

---------

Co-authored-by: Diego Nava <diego.nava77@hotmail.com>

* Fix for possible panic. (#4627)

* Fix, removed duplicated check.

---------

Co-authored-by: Gheis Mohammadi <Gheis.Mohammadi@gmail.com>
Co-authored-by: Diego Nava <diego.nava77@hotmail.com>
Co-authored-by: Konstantin <355847+Frozen@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
high priority high priority issue with customer impact
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants