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

add election backtest #5950

Merged
merged 2 commits into from
Jul 11, 2021

Conversation

zgfzgf
Copy link
Contributor

@zgfzgf zgfzgf commented Apr 3, 2021

./lotus-shed election backtest t01000
height, winCount
223, 5
222, 10
221, 5
220, 7
219, 5
... ...
./lotus-shed election backtest -h
NAME:
lotus-shed election backtest - Backtest elections with given miner

USAGE:
lotus-shed election backtest [command options] [minerAddress]

OPTIONS:
--height value blockchain head height (default: 0)
--count value blockchain count (default: 120)
--help, -h show help (default: false)

Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

2 nits, but this looks useful.

It would also be great to check if the miner had a block in tipsets in which it was eligible to mine, and report if it wasn't present.

Comment on lines 225 to 228
if winner == nil {
return nil, nil
}
return winner, nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Could just be

Suggested change
if winner == nil {
return nil, nil
}
return winner, nil
return winner, nil

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you.

},
&cli.IntFlag{
Name: "count",
Usage: "blockchain count",
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Usage: "blockchain count",
Usage: "number of won elections to look for",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

now OK

@zgfzgf zgfzgf requested a review from arajasek as a code owner May 7, 2021 01:26
@jennijuju jennijuju added the P3 P3: Might get resolved label May 17, 2021
@arajasek
Copy link
Contributor

Thanks for this!

@arajasek arajasek merged commit 93d929a into filecoin-project:master Jul 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 P3: Might get resolved
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants