ProposerVM Epochs POC #3746

cam-schultz · 2025-02-25T23:27:54Z

Why this should be merged

Implements ProposerVM epochs as described in avalanche-foundation/ACPs#181

How this works

Adds a field PChainEpochHeight to proposervm blocks that remains fixed for the epoch's duration. This height is passed to the innerVM block, rather than the windowed P-Chain height.

How this was tested

Added unit test for the epoch calculation, but unit/integration tests for the rest of the changed are still TODO

Need to be documented in RELEASES.md?

Yes

github-actions · 2025-04-13T00:24:31Z

This PR has become stale because it has been open for 30 days with no activity. Adding the lifecycle/frozen label will cause this PR to ignore lifecycle events.

upgrade/upgrade.go

yacovm · 2025-08-15T15:44:41Z

upgrade/upgrade.go

+	EtnaTime                     time.Time     `json:"etnaTime"`
+	FortunaTime                  time.Time     `json:"fortunaTime"`
+	GraniteTime                  time.Time     `json:"graniteTime"`
+	GraniteEpochDuration         time.Duration `json:"graniteEpochDuration"`


shouldn't we add this to Mainnet and Fuji too?

yacovm · 2025-08-15T16:22:17Z

vms/proposervm/block.go

@@ -59,6 +59,9 @@ type Block interface {
 	buildChild(context.Context) (Block, error)

 	pChainHeight(context.Context) (uint64, error)
+	pChainEpochHeight(context.Context) (uint64, error)


what is the use of the context here? Isn't the implementation just memory access?

The Block interface ends up getting used in postForkBlock and postForkOption. postForkOption needs to get the parent block of the "option", which requires the context.

yacovm · 2025-08-15T16:28:55Z

vms/proposervm/block.go

@@ -79,6 +82,51 @@ func (p *postForkCommonComponents) Height() uint64 {
 	return p.innerBlk.Height()
 }

+// Calculates a block's P-Chain epoch height based on its ancestor's epoch membership
+func (p *postForkCommonComponents) getPChainEpoch(ctx context.Context, parentID ids.ID) (uint64, uint64, time.Time, error) {
+	parent, err := p.vm.getBlock(ctx, parentID)


what happens if we call this on the genesis block?

yacovm · 2025-08-15T18:45:01Z

vms/proposervm/block.go

@@ -163,9 +212,23 @@ func (p *postForkCommonComponents) Verify(
 	}

 	var contextPChainHeight uint64
-	if p.vm.Upgrades.IsEtnaActivated(childTimestamp) {
+	switch {
+	case p.vm.Upgrades.IsGraniteActivated(childTimestamp):


what happens if the parent doesn't have granite activated but the child did? How should the system behave?

This is the epochStartTime.IsZero() case inside getPChainEpoch right?

Yes, that's correct. We've change the behaviour to actually check for epoch 0 instead of checking the starttime

yacovm · 2025-08-15T18:48:09Z

vms/proposervm/block.go

+	switch {
+	case p.vm.Upgrades.IsGraniteActivated(childTimestamp):
+		pChainEpochHeight, _, _, err := p.getPChainEpoch(ctx, child.Parent())
+		if err != nil {


We're checking the PchainEpochHeight is what it should be, what about the EpochNumber and EpochStartTime? shouldn't we check that too?

…into proposervm-epochs

yacovm

Not done yet but left some comments in the meantime

yacovm · 2025-08-18T17:42:31Z

tests/e2e/c/proposervm_epoch.go

+	const txAmount = 10 * units.Avax // Arbitrary amount to send and transfer
+
+	ginkgo.It("should advance the proposervm epoch according to the upgrade config epoch duration", func() {
+		// TODO: Skip this test if Granite is not activated


// TODO: Skip this test if Granite is not activated

Why? Can't we force its activation in the test?

We can just do upgrades.GraniteTime = upgrade.InitiallyActiveTime and it will make it activated.

upgrades.GraniteTime gets set outside of the test. The current test suite runs all the tests once without granite activated, and once with them activated.

tests/e2e/c/proposervm_epoch.go

vms/proposervm/service.go

vms/proposervm/client.go

tests/e2e/c/proposervm_epoch.go

yacovm · 2025-08-18T21:47:28Z

vms/proposervm/block.go

@@ -79,6 +82,59 @@ func (p *postForkCommonComponents) Height() uint64 {
 	return p.innerBlk.Height()
 }

+// Calculates a block's P-Chain epoch height based on its ancestor's epoch membership
+func (p *postForkCommonComponents) getPChainEpoch(ctx context.Context, parentID ids.ID) (uint64, uint64, time.Time, error) {


can we:

add the parent block as a parameter to this function instead of retrieving it inside the method

move the logger to be a parameter to the method

make this a function and not an instance method, so it can be easily tested

make a unit test that tests this method directly

?

yacovm · 2025-08-18T21:48:59Z

vms/proposervm/block.go

@@ -89,6 +145,7 @@ func (p *postForkCommonComponents) Height() uint64 {
 // 7) [child]'s timestamp is within its proposer's window
 // 8) [child] has a valid signature from its proposer
 // 9) [child]'s inner block is valid
+// 10) [child] has the expected P-Chain epoch height


what about child has the expected epoch number and epoch start time?

We need to verify every field we encode.

yacovm · 2025-08-18T21:50:59Z

vms/proposervm/block.go

@@ -163,9 +212,23 @@ func (p *postForkCommonComponents) Verify(
 	}

 	var contextPChainHeight uint64
-	if p.vm.Upgrades.IsEtnaActivated(childTimestamp) {
+	switch {
+	case p.vm.Upgrades.IsGraniteActivated(childTimestamp):


This is the epochStartTime.IsZero() case inside getPChainEpoch right?

yacovm · 2025-08-18T21:51:46Z

vms/proposervm/block.go

+			p.vm.ctx.Log.Error("unexpected build verification failure",
+				zap.String("reason", "failed to get P-Chain epoch height"),
+				zap.Stringer("parentID", child.Parent()),
+				zap.Error(err),


don't we need to fail fast and return an error here?

yacovm · 2025-08-18T22:15:16Z

vms/proposervm/block/build.go

+			ParentID:          parentID,
+			Timestamp:         timestamp.Unix(),
+			PChainHeight:      pChainHeight,
+			PChainEpochHeight: pChainEpochHeight,


These new fields are being serialized even if we haven't activated granite yet (they'll be zero), right?

Since the validators don't all upgrade the software version at the same second - how would it work? how can a validator in the current avalanchego version deserialize a block built by a validator in the new avalanchego version?

yacovm · 2025-08-19T15:02:24Z

vms/proposervm/block.go

+				zap.Error(err),
+			)
+		}
+		if childHeight := child.PChainEpochHeight(); pChainEpochHeight != childHeight {


can we extract the ICM epoch attribute validation into a separate function and then just check everything we need there, and not in this function?

yacovm · 2025-08-19T16:06:56Z

vms/proposervm/service.go

+		zap.String("method", "getEpoch"),
+	)
+
+	lastAccepted, err := p.vm.LastAccepted(r.Context())


Isn't this struct supposed to get accessed concurrently? I think we need some kind of synchronization if so.

yacovm · 2025-08-19T16:07:26Z

vms/proposervm/service.go

+		zap.String("encoding", args.Encoding.String()),
+	)
+
+	block, err := p.vm.GetBlock(r.Context(), args.ProposerID)


I am not sure VM access is thread safe

vms/proposervm/service.md

yacovm · 2025-08-19T16:10:29Z

vms/proposervm/service_test.go

+import (
+	"testing"
+	"time"
+)


seems like the tests are missing here.

cam-schultz added 2 commits February 25, 2025 17:24

proposervm epoch poc

f7bca02

verify p-chain epoch height

46f9d06

github-actions bot added the lifecycle/stale label Apr 13, 2025

StephenButtolph removed the lifecycle/stale label May 1, 2025

cam-schultz added 13 commits May 5, 2025 19:07

continuous epoch numbering scheme

5e3941a

first epoch block sets start time

d6873df

Merge branch 'master' into proposervm-epochs

4e8239d

handle initial epoch

5ec65ff

sealing block's timestamp is next epoch's start time

a17f763

Merge branch 'master' into proposervm-epochs

8bcd2b7

Merge branch 'master' into proposervm-epochs

44dd186

e2e skeleton

f70b717

expose proposervm API

d94c0ef

proposervm api client

d4d5d18

client fixes

f52b784

get epoch method

a19bcba

observe advancing epoch

8c3babe

a1k0n reviewed Jul 6, 2025

View reviewed changes

upgrade/upgrade.go Outdated Show resolved Hide resolved

cam-schultz added 6 commits July 7, 2025 14:21

specify epoch as duration

af74732

epoch e2e test

dfaad44

specify epoch as duration

dc586f4

cleanup

6dd2221

update mocks

bd2da2c

lint

d1b1e5b

michaelkaplan13 added the acp181 label Jul 11, 2025

ylg-avalabs and others added 2 commits August 13, 2025 00:44

Merge remote-tracking branch 'origin' into proposervm-epochs

a560a1d

Merge branch 'master' into proposervm-epochs

300032c

yacovm reviewed Aug 15, 2025

View reviewed changes

Merge branch 'master' into proposervm-epochs-e2e

23c634e

cam-schultz and others added 3 commits August 18, 2025 09:42

update mocks

7e7f131

lint

36274a7

Merge branch 'proposervm-epochs' of github.com:ava-labs/avalanche-go …

71fc71b

…into proposervm-epochs

yacovm reviewed Aug 18, 2025

View reviewed changes

yacovm reviewed Aug 19, 2025

View reviewed changes

vms/proposervm/service.md Show resolved Hide resolved

yacovm reviewed Aug 19, 2025

View reviewed changes

vms/proposervm/service_test.go

import (

"testing"

"time"

)

Copy link

Contributor

yacovm Aug 19, 2025

Choose a reason for hiding this comment

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

seems like the tests are missing here.

Merge branch 'master' into proposervm-epochs

06864dc

ProposerVM Epochs POC #3746

Are you sure you want to change the base?

ProposerVM Epochs POC #3746

Uh oh!

Conversation

cam-schultz commented Feb 25, 2025

Why this should be merged

How this works

How this was tested

Need to be documented in RELEASES.md?

Uh oh!

github-actions bot commented Apr 13, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yacovm left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!