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.

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

[vms/proposervm] Update Build Heuristic #2348

Merged
merged 23 commits into from
Nov 22, 2023
Merged

[vms/proposervm] Update Build Heuristic #2348

merged 23 commits into from
Nov 22, 2023

Conversation

patrick-ogrady
Copy link
Contributor

@patrick-ogrady patrick-ogrady commented Nov 20, 2023 •
edited
Loading

This PR updates the heuristic nodes use to build new blocks by hallucinating a "protected window" of 5 minutes (instead of the 30 second window currently used on the verification path that was introduced in Snowman++).

For virtuous nodes, this will delay the start of the "anyone can broadcast" phase to 5 minutes (which should reduce the number of blocks the node must verify when there are gaps in chain production < 5 minutes on VMs with synchronized mempools).

How this was tested

  • Pass existing tests
  • Add new tests (ensure compatibility with Snowman++ verification rules, ensure no virtuous, non-validator will build before MaxBuildDelay)

@patrick-ogrady patrick-ogrady changed the base branch from master to dev November 20, 2023 21:43
@patrick-ogrady patrick-ogrady self-assigned this Nov 21, 2023
@ava-labs ava-labs deleted a comment from abi87 Nov 21, 2023
StephenButtolph
StephenButtolph reviewed Nov 21, 2023
View reviewed changes
Copy link
Contributor

@StephenButtolph StephenButtolph left a comment

Choose a reason for hiding this comment

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

code lgtm

@patrick-ogrady patrick-ogrady marked this pull request as ready for review November 21, 2023 22:18
@patrick-ogrady patrick-ogrady force-pushed the proposervm-build-heuristic branch from 391e608 to 2793e14 Compare November 21, 2023 23:40
StephenButtolph
StephenButtolph reviewed Nov 22, 2023
View reviewed changes
Copy link
Contributor

@StephenButtolph StephenButtolph left a comment

Choose a reason for hiding this comment

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

I don't think the way Timestamps are being set here make much sense for the inner blocks... But tbh they didn't make much sense before either.

I feel like there may have been a bit of a miscommunication around "manually built blocks"... I was referring to direct references to statelessblock.Build and statelessblock.BuildUnsigned. Which I think should use MaxVerifyDelay. I wasn't really referring to the innerVM's blocks (which I feel generally should align with their proposervm header times unless explicitly related to the test).

Regardless - I think other than these few nits this lgtm... I feel like cleaning up all these tests is out of scope for this PR.

vms/proposervm/block_test.go Outdated Show resolved Hide resolved
vms/proposervm/post_fork_block_test.go Outdated Show resolved Hide resolved
vms/proposervm/post_fork_block_test.go Outdated Show resolved Hide resolved
vms/proposervm/proposer/windower.go Outdated Show resolved Hide resolved
@StephenButtolph StephenButtolph added this to the v1.10.16 milestone Nov 22, 2023
@StephenButtolph StephenButtolph added this pull request to the merge queue Nov 22, 2023
Merged via the queue into dev with commit f2f6d0a Nov 22, 2023
16 checks passed
@StephenButtolph StephenButtolph deleted the proposervm-build-heuristic branch November 22, 2023 04:30
joshua-kim added a commit that referenced this pull request Nov 29, 2023
@joshua-kim
Squashed commit of the following:
738bd2e 
commit c5169a3
Author: David Boehm <[email protected]>
Date:   Wed Nov 29 15:34:57 2023 -0500

    MerkleDB Remove ID from Node to reduce size and removal channel creation. (#2324)

    Co-authored-by: Dan Laine <[email protected]>

commit 56c2ad9
Author: Stephen Buttolph <[email protected]>
Date:   Wed Nov 29 14:35:33 2023 -0500

    Replace periodic push accepted gossip with pull preference gossip for block discovery (#2367)

commit 1dddf30
Author: Dhruba Basu <[email protected]>
Date:   Wed Nov 29 07:16:27 2023 -0800

    Rename `D` to `Durango` (#2389)

commit 6ed238c
Author: Stephen Buttolph <[email protected]>
Date:   Wed Nov 29 01:05:16 2023 -0500

    Add block source metrics to monitor gossip (#2386)

commit 21c14b1
Author: Stephen Buttolph <[email protected]>
Date:   Tue Nov 28 23:59:51 2023 -0500

    Add metric to track the stake weight of block providers (#2376)

commit 02ae8d9
Author: Stephen Buttolph <[email protected]>
Date:   Tue Nov 28 15:10:25 2023 -0500

    Unexport RequestID from snowman engine (#2384)

commit be422a0
Author: Stephen Buttolph <[email protected]>
Date:   Tue Nov 28 13:58:45 2023 -0500

    Implement generic bimap (#2383)

commit d7e7ff5
Author: Dhruba Basu <[email protected]>
Date:   Tue Nov 28 10:10:49 2023 -0800

    `vms/avm`: Rename `states` pkg to `state` (#2381)

commit 6e52922
Author: Dhruba Basu <[email protected]>
Date:   Tue Nov 28 07:55:27 2023 -0800

    `vms/platformvm`: Move `toEngine` channel to mempool (#2333)

commit 590ad12
Author: Stephen Buttolph <[email protected]>
Date:   Mon Nov 27 16:57:15 2023 -0500

    Remove common bootstrapper (#2297)

commit 9ad213c
Author: Dhruba Basu <[email protected]>
Date:   Mon Nov 27 11:45:03 2023 -0800

    `vms/platformvm`: Consolidate `state` pkg mocks (#2370)

commit 3a424d0
Author: Stephen Buttolph <[email protected]>
Date:   Mon Nov 27 13:36:03 2023 -0500

    Remove `engine.GetVM` (#2374)

commit b9ab41a
Author: felipemadero <[email protected]>
Date:   Mon Nov 27 14:44:54 2023 -0300

    Add option to provide BLS keys to validators in the genesis (#2371)

commit 42161aa
Author: Dhruba Basu <[email protected]>
Date:   Mon Nov 27 09:07:48 2023 -0800

    `vms/platformvm`: Adjust `Diff.Apply` signature (#2368)

commit 79e572d
Author: felipemadero <[email protected]>
Date:   Mon Nov 27 13:43:32 2023 -0300

    Avoid closing stdout and stderr during log close (#2372)

commit e04dad8
Author: Joshua Kim <[email protected]>
Date:   Sun Nov 26 13:13:56 2023 -0500

    Move peerTracker from x/sync to network/p2p (#2356)

commit b1b051a
Author: Dhruba Basu <[email protected]>
Date:   Sat Nov 25 16:32:05 2023 -0800

    `vms/platformvm`: Remove unused `withMetrics` txheap (#2373)

commit 8d9b93c
Author: Stephen Buttolph <[email protected]>
Date:   Fri Nov 24 12:45:51 2023 -0500

    Add metric for duration between block timestamp and block acceptance time (#2366)

commit 9353569
Author: Stephen Buttolph <[email protected]>
Date:   Fri Nov 24 11:23:36 2023 -0500

    Document storage growth in readme (#2364)

commit 6ad31d6
Author: Dhruba Basu <[email protected]>
Date:   Wed Nov 22 17:40:43 2023 -0800

    Remove Banff check from mempool verifier (#2360)

commit 62df19c
Author: Stephen Buttolph <[email protected]>
Date:   Wed Nov 22 19:47:37 2023 -0500

    Update versions for v1.10.16 (#2353)

commit 48c541c
Author: Stephen Buttolph <[email protected]>
Date:   Wed Nov 22 17:46:14 2023 -0500

    Fix P-chain mempool tx count metric (#2361)

commit 4ce0d67
Author: Gyuho Lee <[email protected]>
Date:   Thu Nov 23 01:39:53 2023 +0800

    Use linkedhashmap for P-Chain mempool (#1536)

    Co-authored-by: dhrubabasu <[email protected]>
    Co-authored-by: Stephen Buttolph <[email protected]>

commit f2f6d0a
Author: Patrick O'Grady <[email protected]>
Date:   Tue Nov 21 22:07:24 2023 -0600

    [vms/proposervm] Update Build Heuristic (#2348)

    Co-authored-by: Alberto Benegiamo <[email protected]>

commit 459f8ba
Author: Stephen Buttolph <[email protected]>
Date:   Tue Nov 21 19:24:02 2023 -0500

    Reduce the size of the OracleBlock interface (#2355)

commit f7cc69b
Author: Stephen Buttolph <[email protected]>
Date:   Tue Nov 21 17:13:17 2023 -0500

    Expand consensus health check (#2354)

commit 2b737d5
Author: Stephen Buttolph <[email protected]>
Date:   Tue Nov 21 13:26:55 2023 -0500

    Cleanup snowman consensus metrics (#2349)

Signed-off-by: Joshua Kim <[email protected]>
@abi87 abi87 mentioned this pull request Dec 5, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@StephenButtolph StephenButtolph StephenButtolph approved these changes

@dhrubabasu dhrubabasu dhrubabasu approved these changes

@abi87 abi87 Awaiting requested review from abi87

@aaronbuchwald aaronbuchwald Awaiting requested review from aaronbuchwald

Assignees

@patrick-ogrady patrick-ogrady

Labels
None yet
Projects
Platform Engineering Group
Archived in project
Milestone
v1.10.16
Development

Successfully merging this pull request may close these issues.
4 participants
@patrick-ogrady @dhrubabasu @abi87 @StephenButtolph