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

[NIT-2489] Honest Strategy Revamp to Consider Path Weights #634

Merged
merged 85 commits into from
Nov 22, 2024

Conversation

rauljordan
Copy link
Collaborator

@rauljordan rauljordan commented May 3, 2024

Fixes: NIT-2489

Key reading:

Background:
This PR changes the honest strategy to match the expected specification linked above. The core idea is that edge trackers should only attempt to bisect or open subchallenges if the weight of timers to their closest root ancestor is < 1 challenge period.

This means we also need to change the rules for when an edge tracker should "despawn". The changes are as follows:

If the edge is an essential root => despawn once it is confirmed.
else if not an essential root  => 
	if edge is small step and length one => despawn once confirmed by OSP
	else: despawn once the closest essential ancestor root is confirmed

Terminology remapping:

  • Path weight: the sum of the local timers along a path of edges up to some root edge
  • Proof node: any lowest level subchallenge edge of length 1

This PR also adds an e2e test that checks that all essential root edges are confirmed, beyond just the challenge completing.

@rauljordan rauljordan force-pushed the honest-strategy-changes branch 2 times, most recently from 4bb65bd to 1cfe2fd Compare September 20, 2024 15:39
@rauljordan rauljordan changed the title update how root nodes are confirmed Honest Strategy Revamp to Consider Path Weights Sep 20, 2024
@eljobe eljobe self-assigned this Oct 16, 2024
@amsanghi amsanghi changed the base branch from is-essential-confirmable to main November 7, 2024 10:38
@amsanghi amsanghi changed the base branch from main to is-essential-confirmable November 7, 2024 10:46
@amsanghi amsanghi changed the base branch from is-essential-confirmable to main November 7, 2024 11:18
@amsanghi amsanghi assigned amsanghi and unassigned eljobe Nov 14, 2024
@amsanghi amsanghi changed the title Honest Strategy Revamp to Consider Path Weights [NIT-2489] Honest Strategy Revamp to Consider Path Weights Nov 14, 2024
@amsanghi amsanghi requested a review from eljobe November 14, 2024 16:03
Copy link
Member

@eljobe eljobe left a comment

Choose a reason for hiding this comment

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

In the description of the PR, Raul suggests that we should also be checking in the e2e_test.go that the essential root edges are confirmed in addition to winning the challenge, can you take a stab at that as well?

Mostly, this PR is looking really good though. I especially like the simplification of getting rid of the min heap.

challenge-manager/chain-watcher/watcher.go Outdated Show resolved Hide resolved
@rauljordan rauljordan marked this pull request as draft November 21, 2024 20:40
@rauljordan rauljordan marked this pull request as ready for review November 21, 2024 20:40
@rauljordan
Copy link
Collaborator Author

Note @eljobe , the big diff is due to updating MODULE.bazel.lock. The real diff is still around +1000

Copy link
Member

@eljobe eljobe left a comment

Choose a reason for hiding this comment

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

LGTM

With the current timings, it usually takes a little bit to long to fit in the
"medium" size category.
@eljobe eljobe merged commit 218c2a3 into main Nov 22, 2024
5 checks passed
@eljobe eljobe deleted the honest-strategy-changes branch November 22, 2024 13:47
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