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

Drop Pending Stakers 5 - validated PostDurango StakerTxs #2314

Merged
merged 239 commits into from
Dec 18, 2023
Merged

Drop Pending Stakers 5 - validated PostDurango StakerTxs #2314

merged 239 commits into from
Dec 18, 2023

Conversation

abi87
Copy link
Contributor

@abi87 abi87 commented Nov 15, 2023 •
edited

Why this should be merged

This is the main PR containing the drop of pending validators.
AddStakerTx.StartTime is not anymore validated by itself; instead staking duration (AddStakerTx.EndTime - chainTime) is validated. Stakers are immediately marked as current instead of pending ones.

How this works

Slicing #2175 up

How this was tested

abi87
abi87 commented Nov 15, 2023
View reviewed changes
vms/platformvm/validator_set_property_test.go Outdated
Comment on lines 395 to 413
// move time ahead, promoting the validator to current
currentTime := stakerTx.StartTime()
vm.clock.Set(currentTime)
vm.state.SetTimestamp(currentTime)

blk, err = vm.Builder.BuildBlock(context.Background())
if err != nil {
return nil, fmt.Errorf("failed building block: %w", err)
}
if err := blk.Verify(context.Background()); err != nil {
return nil, fmt.Errorf("failed verifying block: %w", err)
}
if err := blk.Accept(context.Background()); err != nil {
return nil, fmt.Errorf("failed accepting block: %w", err)
}
if err := vm.SetPreference(context.Background(), vm.manager.LastAccepted()); err != nil {
return nil, fmt.Errorf("failed setting preference: %w", err)
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Durango active here, no more pending

@abi87 abi87 changed the title Validating PostDurango StakerTxs Drop Pending Stakers - validated PostDurango StakerTxs Nov 15, 2023
@abi87 abi87 force-pushed the post_durango_stakerTxs_verification branch from 95b949e to 8266e5e Compare November 15, 2023 11:17
@abi87 abi87 self-assigned this Nov 15, 2023
@abi87 abi87 changed the title Drop Pending Stakers - validated PostDurango StakerTxs Drop Pending Stakers 3 - validated PostDurango StakerTxs Nov 15, 2023
abi87
abi87 commented Nov 15, 2023
View reviewed changes
vms/platformvm/service_test.go Outdated
@@ -73,7 +73,7 @@ var (
)

func defaultService(t *testing.T) (*Service, *mutableSharedMemory) {
vm, _, mutableSharedMemory := defaultVM(t)
vm, _, mutableSharedMemory := defaultVM(t, latestFork)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

P-chain UTs do not handle correctly forks at the moment.
Where is a bunch of PRs geared to fix this (see #2169).
While we wait to have them reviewed and merged, this is the minimal amount of changes I needed to have this PR tested meaningfully.

@abi87 abi87 force-pushed the post_durango_stakerTxs_verification branch from 527251b to eadfbe1 Compare November 15, 2023 13:40
abi87
abi87 commented Nov 15, 2023
View reviewed changes
vms/platformvm/txs/executor/staker_tx_verification.go Outdated
// to allow the verifier visitor to explicitly check for this error.
maxStartTime := currentTimestamp.Add(MaxFutureStartTime)
if preDurangoStartTime.After(maxStartTime) {
return nil, ErrFutureStakeTime
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we must guarantee this is the last error that occurs, if an error occurs.
Should be possible to cleanup post fork actiation

abi87
abi87 commented Nov 15, 2023
View reviewed changes
vms/platformvm/txs/validator.go
@@ -40,11 +40,6 @@ func (v *Validator) EndTime() time.Time {
return time.Unix(int64(v.End), 0)
}

// Duration is the amount of time that this validator will be in the validator set
func (v *Validator) Duration() time.Duration {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Post Durango this is evil. Start and End are there for backward compatiblity reasons, but result has nothing to do with staking duration. Better bury this method

Base automatically changed from post_durango_minimal_test_infra to dev December 15, 2023 23:34
@StephenButtolph StephenButtolph mentioned this pull request Dec 18, 2023
Merged
1 task
@dhrubabasu dhrubabasu mentioned this pull request Dec 18, 2023
Closed
2 tasks
@StephenButtolph StephenButtolph added this pull request to the merge queue Dec 18, 2023
Merged via the queue into dev with commit aa764bb Dec 18, 2023
17 checks passed
@StephenButtolph StephenButtolph deleted the post_durango_stakerTxs_verification branch December 18, 2023 23:13
joshua-kim added a commit that referenced this pull request Dec 19, 2023
@joshua-kim
Squashed commit of the following:
c8866e6 
commit 7a0dfda
Author: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Date:   Tue Dec 12 03:29:47 2023 -0500

    testing structs

    Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>

commit 58d2779
Author: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com>
Date:   Tue Dec 19 14:01:16 2023 -0500

    Dedupe secp256k1 key usage in tests (#2511)

commit 6c64372
Author: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com>
Date:   Tue Dec 19 11:45:58 2023 -0500

    Remove unused `AcceptorTracker` struct (#2508)

commit 7a9bdad
Author: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com>
Date:   Tue Dec 19 10:44:20 2023 -0500

    Remove unused `BuildGenesisTest` function (#2503)

commit 44d71db
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Dec 18 19:05:26 2023 -0500

    Bump golang.org/x/crypto from 0.14.0 to 0.17.0 (#2502)

    Signed-off-by: dependabot[bot] <support@github.com>
    Signed-off-by: Stephen Buttolph <stephen@avalabs.org>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
    Co-authored-by: Stephen Buttolph <stephen@avalabs.org>

commit aa764bb
Author: Alberto Benegiamo <alberto.benegiamo@gmail.com>
Date:   Mon Dec 18 16:00:17 2023 -0700

    Drop Pending Stakers 5 - validated PostDurango StakerTxs (#2314)

    Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
    Co-authored-by: dhrubabasu <7675102+dhrubabasu@users.noreply.github.com>

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
joshua-kim added a commit that referenced this pull request Dec 19, 2023
@joshua-kim
Squashed commit of the following:
23f7cdd 
commit 7a0dfda
Author: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Date:   Tue Dec 12 03:29:47 2023 -0500

    testing structs

    Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>

commit 58d2779
Author: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com>
Date:   Tue Dec 19 14:01:16 2023 -0500

    Dedupe secp256k1 key usage in tests (#2511)

commit 6c64372
Author: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com>
Date:   Tue Dec 19 11:45:58 2023 -0500

    Remove unused `AcceptorTracker` struct (#2508)

commit 7a9bdad
Author: Dhruba Basu <7675102+dhrubabasu@users.noreply.github.com>
Date:   Tue Dec 19 10:44:20 2023 -0500

    Remove unused `BuildGenesisTest` function (#2503)

commit 44d71db
Author: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Date:   Mon Dec 18 19:05:26 2023 -0500

    Bump golang.org/x/crypto from 0.14.0 to 0.17.0 (#2502)

    Signed-off-by: dependabot[bot] <support@github.com>
    Signed-off-by: Stephen Buttolph <stephen@avalabs.org>
    Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
    Co-authored-by: Stephen Buttolph <stephen@avalabs.org>

commit aa764bb
Author: Alberto Benegiamo <alberto.benegiamo@gmail.com>
Date:   Mon Dec 18 16:00:17 2023 -0700

    Drop Pending Stakers 5 - validated PostDurango StakerTxs (#2314)

    Co-authored-by: Stephen Buttolph <stephen@avalabs.org>
    Co-authored-by: dhrubabasu <7675102+dhrubabasu@users.noreply.github.com>

Signed-off-by: Joshua Kim <20001595+joshua-kim@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhrubabasu dhrubabasu dhrubabasu approved these changes

@StephenButtolph StephenButtolph StephenButtolph approved these changes

@gyuho gyuho Awaiting requested review from gyuho gyuho is a code owner

@danlaine danlaine Awaiting requested review from danlaine danlaine is a code owner

@marun marun Awaiting requested review from marun marun is a code owner

Assignees

@abi87 abi87

Labels
Durango durango fork
Projects
Platform Engineering Group
Archived in project
Milestone
v1.10.18
Development

Successfully merging this pull request may close these issues.

Drop Pending Stakers
4 participants
@abi87 @marun @dhrubabasu @StephenButtolph