[Merged by Bors] - Give NiPoSTBuilder more time to create the PoST #4893

fasmat · 2023-08-22T11:36:08Z

Motivation

With this change ATXs can now be published until the end of their PublishEpoch (instead of until the start of the next PoET round). On mainnet this means a node as 4 additional days of time to generate a PoST proof and publish an ATX.

It is still required to publishing within the window between the end of one PoET round and the start of the next one to not miss any rewards. This change only allows nodes that weren't able to register for the next PoET round to still publish an ATX within the 4 days (on mainnet) between the end of the CycleGap and the end of the PublishEpoch.

Changes

This PR adapts deadlines to the following:

Phase 0 (submitting challenge to PoET): deadline poetRoundStart - unchanged
Phase 1 (fetching proof from PoET): before the deadline was the end of the round it submitted to + a grace period of 1 hour; this effectively means there is only a 1 hour window to fetch the proof from PoET before the node gives up. Now instead it will fetch until the end of the PublishEpoch. This means that if the node doesn't manage to publish its ATX before next PoET round starts, it won't be able to publish one next epoch (unchanged) but will still try to publish one for the current epoch.
Phase 2 (publishing the ATX): before deadline was nextPoetRoundStart, with the intention to publish the ATX and have enough time left to register for the next PoET round. This is a very short window (just the time between the end of one PoET round and the start of the next one). If the node doesn't manage to generate a PoST proof within that time it gives up on publishing an ATX and misses out on rewards it could get by instead giving up on registering on the next PoET round. This deadline was also extended to the end of the PublishEpoch.

Test Plan

existing tests pass or have been adapted

TODO

Explain motivation or link existing issue(s)
Test changes and document test plan
Update documentation as needed

DevOps Notes

This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources)
This PR does not affect public APIs
This PR does not rely on a new version of external services (PoET, elasticsearch, etc.)
This PR does not make changes to log messages (which monitoring infrastructure may rely on)

codecov · 2023-08-22T12:20:10Z

Codecov Report

Merging #4893 (a0861e4) into develop (4bd907c) will decrease coverage by 0.1%.
The diff coverage is 88.0%.

@@            Coverage Diff            @@
##           develop   #4893     +/-   ##
=========================================
- Coverage     76.8%   76.7%   -0.1%     
=========================================
  Files          256     256             
  Lines        29632   29635      +3     
=========================================
- Hits         22759   22755      -4     
- Misses        5402    5408      +6     
- Partials      1471    1472      +1

Files Changed	Coverage Δ
activation/activation.go	`75.4% <50.0%> (-0.6%)`	⬇️
activation/nipost.go	`86.0% <100.0%> (+1.7%)`	⬆️

... and 4 files with indirect coverage changes

fasmat · 2023-08-22T12:29:44Z

bors try

activation/nipost.go

bors · 2023-08-22T13:20:29Z

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

fasmat · 2023-08-22T16:58:59Z

bors try

bors · 2023-08-22T17:48:36Z

try

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

activation/nipost.go

fasmat · 2023-08-23T08:50:57Z

bors merge

## Motivation Closes #4797 ## Changes Phase 2 of the BuildNiPoST step in publishing an ATX is restricted with a deadline of `nextPoetRoundStart`. This deadline doesn't need to be this strict. This PR adapts deadlines to the following: - Phase 0 (submitting challenge to PoET): timeout `poetRoundStart` - unchanged - Phase 1 (fetching proof from PoET): before the deadline was the end of the round round it submitted to + a grace period of 1 hour; this effectively means there is only a 1 hour window to fetch the proof from PoET before the node gives up. Now instead it will fetch with a deadline until the end of the next PoET round. This means that if the node doesn't manage to publish its ATX before next PoET round starts, it won't be able to publish one next epoch (unchanged) but will still try to publish one for the current epoch, even if its late. It will still miss at most 1 epoch (when its late). - Phase 2 (publishing the ATX): before timeout was `nextPoetRoundStart`, with the intention to publish the ATX and have enough time left to register for the next PoET round. This is a very short window (just the time between the end of one PoET round and the start of the next one). If the node doesn't manage to generate a PoST proof within that time it gives up on publishing an ATX and misses out on rewards it could get by instead giving up on registering on the next PoET round. This gives the last phase ~ 2 weeks of time instead of just a few hours, but if the node takes longer than `nextPoetRoundStart` it will not be able to submit a challenge for the next PoET round in time. This is because the new challenge contains the ID of the ATX that is being built right now. In that case the node will skip publishing an ATX for one epoch. ## Test Plan existing tests pass or have been adapted ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Update documentation as needed ## DevOps Notes  - [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources) - [x] This PR does not affect public APIs - [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.) - [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

bors · 2023-08-23T09:39:16Z

Build failed (retrying...):

systest-status

## Motivation Closes #4797 ## Changes Phase 2 of the BuildNiPoST step in publishing an ATX is restricted with a deadline of `nextPoetRoundStart`. This deadline doesn't need to be this strict. This PR adapts deadlines to the following: - Phase 0 (submitting challenge to PoET): timeout `poetRoundStart` - unchanged - Phase 1 (fetching proof from PoET): before the deadline was the end of the round round it submitted to + a grace period of 1 hour; this effectively means there is only a 1 hour window to fetch the proof from PoET before the node gives up. Now instead it will fetch with a deadline until the end of the next PoET round. This means that if the node doesn't manage to publish its ATX before next PoET round starts, it won't be able to publish one next epoch (unchanged) but will still try to publish one for the current epoch, even if its late. It will still miss at most 1 epoch (when its late). - Phase 2 (publishing the ATX): before timeout was `nextPoetRoundStart`, with the intention to publish the ATX and have enough time left to register for the next PoET round. This is a very short window (just the time between the end of one PoET round and the start of the next one). If the node doesn't manage to generate a PoST proof within that time it gives up on publishing an ATX and misses out on rewards it could get by instead giving up on registering on the next PoET round. This gives the last phase ~ 2 weeks of time instead of just a few hours, but if the node takes longer than `nextPoetRoundStart` it will not be able to submit a challenge for the next PoET round in time. This is because the new challenge contains the ID of the ATX that is being built right now. In that case the node will skip publishing an ATX for one epoch. ## Test Plan existing tests pass or have been adapted ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Update documentation as needed ## DevOps Notes  - [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources) - [x] This PR does not affect public APIs - [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.) - [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

dshulyak · 2023-08-23T09:54:09Z

bors cancel

bors · 2023-08-23T09:54:11Z

Canceled.

fasmat · 2023-08-23T12:44:21Z

bors try

bors · 2023-08-23T13:07:15Z

try

Build failed:

ci-status

activation/nipost.go

fasmat · 2023-08-23T17:22:19Z

bors merge

## Motivation Closes #4797 With this change ATXs can now be published until the end of their `PublishEpoch` (instead of until the start of the next PoET round). On mainnet this means a node as 4 additional days of time to generate a PoST proof and publish an ATX. It is still required to publishing within the window between the end of one PoET round and the start of the next one to not miss any rewards. This change only allows nodes that weren't able to register for the next PoET round to still publish an ATX within the 4 days (on mainnet) between the end of the `CycleGap` and the end of the `PublishEpoch`. ## Changes This PR adapts deadlines to the following: - Phase 0 (submitting challenge to PoET): deadline `poetRoundStart` - unchanged - Phase 1 (fetching proof from PoET): before the deadline was the end of the round it submitted to + a grace period of 1 hour; this effectively means there is only a 1 hour window to fetch the proof from PoET before the node gives up. Now instead it will fetch until the end of the `PublishEpoch`. This means that if the node doesn't manage to publish its ATX before next PoET round starts, it won't be able to publish one next epoch (unchanged) but will still try to publish one for the current epoch. - Phase 2 (publishing the ATX): before deadline was `nextPoetRoundStart`, with the intention to publish the ATX and have enough time left to register for the next PoET round. This is a very short window (just the time between the end of one PoET round and the start of the next one). If the node doesn't manage to generate a PoST proof within that time it gives up on publishing an ATX and misses out on rewards it could get by instead giving up on registering on the next PoET round. This deadline was also extended to the end of the `PublishEpoch`. ## Test Plan existing tests pass or have been adapted ## TODO  - [x] Explain motivation or link existing issue(s) - [x] Test changes and document test plan - [x] Update documentation as needed ## DevOps Notes  - [x] This PR does not require configuration changes (e.g., environment variables, GitHub secrets, VM resources) - [x] This PR does not affect public APIs - [x] This PR does not rely on a new version of external services (PoET, elasticsearch, etc.) - [x] This PR does not make changes to log messages (which monitoring infrastructure may rely on)

bors · 2023-08-23T18:08:37Z

Pull request successfully merged into develop.

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

fasmat self-assigned this Aug 22, 2023

fasmat requested review from countvonzero, dshulyak and poszu as code owners August 22, 2023 11:36

fasmat changed the title ~~Give NiPoSTBuilder more time to for phase 2~~ Give NiPoSTBuilder more time for phase 2 Aug 22, 2023

bors bot added a commit that referenced this pull request Aug 22, 2023

Try #4893:

7eded6a

poszu reviewed Aug 22, 2023

View reviewed changes

activation/nipost.go Outdated Show resolved Hide resolved

bors bot added a commit that referenced this pull request Aug 22, 2023

Try #4893:

9af2ade

fasmat changed the title ~~Give NiPoSTBuilder more time for phase 2~~ Give NiPoSTBuilder more time to create the PoST Aug 22, 2023

poszu approved these changes Aug 23, 2023

View reviewed changes

activation/nipost.go Outdated Show resolved Hide resolved

poszu self-requested a review August 23, 2023 07:46

poszu approved these changes Aug 23, 2023

View reviewed changes

fasmat added 7 commits August 23, 2023 10:50

Give NiPoSTBuilder more time to for phase 2

60280d4

Revert to old timeout checks

7dcd061

Cleanup

1591fb6

Cleanup

beb67b3

WiP

91d4bfb

Remove obsolete test

a112c6e

Add timeout for createAtx

e8b015b

fasmat added 5 commits August 23, 2023 10:50

Add test

c6ad2e8

Fix failing test

23e9a63

Updated naming

1f19eae

Update Changelog

682d2fc

Ensure ATX is published before the end of publish epoch

4665109

fasmat force-pushed the 4797-fix-atx-timeouts-in-builder branch from 2a472ad to 4665109 Compare August 23, 2023 12:04

Update last deadline

bfd15ba

bors bot added a commit that referenced this pull request Aug 23, 2023

Try #4893:

3989438

fasmat requested a review from poszu August 23, 2023 12:56

poszu approved these changes Aug 23, 2023

View reviewed changes

activation/nipost.go Outdated Show resolved Hide resolved

Drop grace period

a0861e4

bors bot changed the title ~~Give NiPoSTBuilder more time to create the PoST~~ [Merged by Bors] - Give NiPoSTBuilder more time to create the PoST Aug 23, 2023

bors bot closed this Aug 23, 2023

bors bot deleted the 4797-fix-atx-timeouts-in-builder branch August 23, 2023 18:08

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Merged by Bors] - Give NiPoSTBuilder more time to create the PoST #4893

[Merged by Bors] - Give NiPoSTBuilder more time to create the PoST #4893

fasmat commented Aug 22, 2023 •

edited

codecov bot commented Aug 22, 2023 •

edited

fasmat commented Aug 22, 2023

bors bot commented Aug 22, 2023

fasmat commented Aug 22, 2023

bors bot commented Aug 22, 2023

fasmat commented Aug 23, 2023

bors bot commented Aug 23, 2023

dshulyak commented Aug 23, 2023

bors bot commented Aug 23, 2023

fasmat commented Aug 23, 2023

bors bot commented Aug 23, 2023

fasmat commented Aug 23, 2023

bors bot commented Aug 23, 2023

[Merged by Bors] - Give NiPoSTBuilder more time to create the PoST #4893

[Merged by Bors] - Give NiPoSTBuilder more time to create the PoST #4893

Conversation

fasmat commented Aug 22, 2023 • edited

Motivation

Changes

Test Plan

TODO

DevOps Notes

codecov bot commented Aug 22, 2023 • edited

Codecov Report

fasmat commented Aug 22, 2023

bors bot commented Aug 22, 2023

try

fasmat commented Aug 22, 2023

bors bot commented Aug 22, 2023

try

fasmat commented Aug 23, 2023

bors bot commented Aug 23, 2023

dshulyak commented Aug 23, 2023

bors bot commented Aug 23, 2023

fasmat commented Aug 23, 2023

bors bot commented Aug 23, 2023

try

fasmat commented Aug 23, 2023

bors bot commented Aug 23, 2023

fasmat commented Aug 22, 2023 •

edited

codecov bot commented Aug 22, 2023 •

edited