-
Notifications
You must be signed in to change notification settings - Fork 779
ProposerVM Epochs POC #3746
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
base: master
Are you sure you want to change the base?
ProposerVM Epochs POC #3746
Changes from all commits
f7bca02
46f9d06
5e3941a
d6873df
4e8239d
5ec65ff
a17f763
8bcd2b7
44dd186
f70b717
d94c0ef
d4d5d18
f52b784
a19bcba
8c3babe
af74732
dfaad44
dc586f4
6dd2221
bd2da2c
d1b1e5b
a560a1d
300032c
23c634e
7e7f131
36274a7
71fc71b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,100 @@ | ||
// Copyright (C) 2019-2024, Ava Labs, Inc. All rights reserved. | ||
// See the file LICENSE for licensing terms. | ||
|
||
package c | ||
|
||
import ( | ||
"math/big" | ||
"time" | ||
|
||
"github.com/ava-labs/libevm/core/types" | ||
"github.com/onsi/ginkgo/v2" | ||
"github.com/stretchr/testify/require" | ||
"go.uber.org/zap" | ||
|
||
"github.com/ava-labs/avalanchego/tests/fixture/e2e" | ||
"github.com/ava-labs/avalanchego/utils/units" | ||
"github.com/ava-labs/avalanchego/vms/proposervm" | ||
) | ||
|
||
var _ = e2e.DescribeCChain("[ProposerVM Epoch]", func() { | ||
tc := e2e.NewTestContext() | ||
require := require.New(tc) | ||
|
||
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 | ||
|
||
env := e2e.GetEnv(tc) | ||
var ( | ||
senderKey = env.PreFundedKey | ||
senderEthAddress = senderKey.EthAddress() | ||
recipientKey = e2e.NewPrivateKey(tc) | ||
recipientEthAddress = recipientKey.EthAddress() | ||
) | ||
|
||
tc.By("initializing a new eth client") | ||
// Select a random node URI to use for both the eth client and | ||
// the wallet to avoid having to verify that all nodes are at | ||
// the same height before initializing the wallet. | ||
nodeURI := env.GetRandomNodeURI() | ||
ethClient := e2e.NewEthClient(tc, nodeURI) | ||
|
||
proposerClient := proposervm.NewClient(nodeURI.URI, "C") | ||
|
||
tc.By("issuing C-Chain transactions to advance the epoch", func() { | ||
// Issue enough C-Chain transactions to observe the epoch advancing | ||
ticker := time.NewTicker(1 * time.Second) | ||
defer ticker.Stop() | ||
|
||
// Issue enough txs to activate the proposervm form and ensure we advance the epoch (duration is 4s) | ||
const numTxs = 7 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if the epoch duration is 4 seconds why do we need 7 transactions separated by a second? Can't we do 1 transaction, wait for it to be finalized, sleep 4 seconds, then another transaction? |
||
txCount := 0 | ||
|
||
initialEpochNumber, _, _, err := proposerClient.GetEpoch(tc.DefaultContext()) | ||
require.NoError(err) | ||
|
||
for range ticker.C { | ||
acceptedNonce, err := ethClient.AcceptedNonceAt(tc.DefaultContext(), senderEthAddress) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These lines aren't a critical part of the test, they're just for sending a transaction to advance the chain. Can we extract the lines 59-79 to a separate function? |
||
require.NoError(err) | ||
gasPrice := e2e.SuggestGasPrice(tc, ethClient) | ||
tx := types.NewTransaction( | ||
acceptedNonce, | ||
recipientEthAddress, | ||
big.NewInt(int64(txAmount)), | ||
e2e.DefaultGasLimit, | ||
gasPrice, | ||
nil, | ||
) | ||
|
||
// Sign transaction | ||
cChainID, err := ethClient.ChainID(tc.DefaultContext()) | ||
require.NoError(err) | ||
signer := types.NewEIP155Signer(cChainID) | ||
signedTx, err := types.SignTx(tx, signer, senderKey.ToECDSA()) | ||
require.NoError(err) | ||
|
||
receipt := e2e.SendEthTransaction(tc, ethClient, signedTx) | ||
require.Equal(types.ReceiptStatusSuccessful, receipt.Status) | ||
|
||
epochNumber, epochStartTime, pChainHeight, err := proposerClient.GetEpoch(tc.DefaultContext()) | ||
Check failure on line 81 in tests/e2e/c/proposervm_epoch.go
|
||
tc.Log().Debug( | ||
"epoch", | ||
zap.Uint64("Epoch Number:", epochNumber), | ||
zap.Uint64("Epoch Start Time:", epochStartTime), | ||
zap.Uint64("P-Chain Height:", pChainHeight), | ||
) | ||
|
||
txCount++ | ||
if txCount >= numTxs { | ||
require.Greater(epochNumber, initialEpochNumber, | ||
"expected epoch number to advance after issuing %d transactions, but it did not", | ||
numTxs, | ||
) | ||
break | ||
} | ||
} | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -77,28 +77,30 @@ var ( | |
EtnaTime: InitiallyActiveTime, | ||
FortunaTime: InitiallyActiveTime, | ||
GraniteTime: UnscheduledActivationTime, | ||
GraniteEpochDuration: 30 * time.Second, | ||
} | ||
|
||
ErrInvalidUpgradeTimes = errors.New("invalid upgrade configuration") | ||
) | ||
|
||
type Config struct { | ||
ApricotPhase1Time time.Time `json:"apricotPhase1Time"` | ||
ApricotPhase2Time time.Time `json:"apricotPhase2Time"` | ||
ApricotPhase3Time time.Time `json:"apricotPhase3Time"` | ||
ApricotPhase4Time time.Time `json:"apricotPhase4Time"` | ||
ApricotPhase4MinPChainHeight uint64 `json:"apricotPhase4MinPChainHeight"` | ||
ApricotPhase5Time time.Time `json:"apricotPhase5Time"` | ||
ApricotPhasePre6Time time.Time `json:"apricotPhasePre6Time"` | ||
ApricotPhase6Time time.Time `json:"apricotPhase6Time"` | ||
ApricotPhasePost6Time time.Time `json:"apricotPhasePost6Time"` | ||
BanffTime time.Time `json:"banffTime"` | ||
CortinaTime time.Time `json:"cortinaTime"` | ||
CortinaXChainStopVertexID ids.ID `json:"cortinaXChainStopVertexID"` | ||
DurangoTime time.Time `json:"durangoTime"` | ||
EtnaTime time.Time `json:"etnaTime"` | ||
FortunaTime time.Time `json:"fortunaTime"` | ||
GraniteTime time.Time `json:"graniteTime"` | ||
ApricotPhase1Time time.Time `json:"apricotPhase1Time"` | ||
ApricotPhase2Time time.Time `json:"apricotPhase2Time"` | ||
ApricotPhase3Time time.Time `json:"apricotPhase3Time"` | ||
ApricotPhase4Time time.Time `json:"apricotPhase4Time"` | ||
ApricotPhase4MinPChainHeight uint64 `json:"apricotPhase4MinPChainHeight"` | ||
ApricotPhase5Time time.Time `json:"apricotPhase5Time"` | ||
ApricotPhasePre6Time time.Time `json:"apricotPhasePre6Time"` | ||
ApricotPhase6Time time.Time `json:"apricotPhase6Time"` | ||
ApricotPhasePost6Time time.Time `json:"apricotPhasePost6Time"` | ||
BanffTime time.Time `json:"banffTime"` | ||
CortinaTime time.Time `json:"cortinaTime"` | ||
CortinaXChainStopVertexID ids.ID `json:"cortinaXChainStopVertexID"` | ||
DurangoTime time.Time `json:"durangoTime"` | ||
EtnaTime time.Time `json:"etnaTime"` | ||
FortunaTime time.Time `json:"fortunaTime"` | ||
GraniteTime time.Time `json:"graniteTime"` | ||
GraniteEpochDuration time.Duration `json:"graniteEpochDuration"` | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't we add this to |
||
} | ||
|
||
func (c *Config) Validate() error { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -59,6 +59,9 @@ type Block interface { | |
buildChild(context.Context) (Block, error) | ||
|
||
pChainHeight(context.Context) (uint64, error) | ||
pChainEpochHeight(context.Context) (uint64, error) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what is the use of the context here? Isn't the implementation just memory access? |
||
epochNumber(context.Context) (uint64, error) | ||
epochStartTime(context.Context) (time.Time, error) | ||
} | ||
|
||
type PostForkBlock interface { | ||
|
@@ -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) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we:
? |
||
parent, err := p.vm.getBlock(ctx, parentID) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what happens if we call this on the genesis block? |
||
if err != nil { | ||
return 0, 0, time.Time{}, err | ||
} | ||
parentTimestamp := parent.Timestamp() | ||
epoch, err := parent.epochNumber(ctx) | ||
if err != nil { | ||
return 0, 0, time.Time{}, fmt.Errorf("failed to get epoch number: %w", err) | ||
} | ||
epochStartTime, err := parent.epochStartTime(ctx) | ||
if err != nil { | ||
return 0, 0, time.Time{}, fmt.Errorf("failed to get epoch start time: %w", err) | ||
} | ||
if epochStartTime.IsZero() { | ||
// If the parent was not assigned an epoch, then the child is the first block of | ||
// the initial epoch. | ||
height, err := parent.pChainHeight(ctx) | ||
if err != nil { | ||
return 0, 0, time.Time{}, fmt.Errorf("failed to get P-Chain height: %w", err) | ||
} | ||
return height, 0, parentTimestamp, nil | ||
} | ||
|
||
if parentTimestamp.After(epochStartTime.Add(p.vm.Upgrades.GraniteEpochDuration)) { | ||
// If the parent crossed the epoch boundary, then it sealed the previous epoch. The child | ||
// is the first block of the new epoch, so should use the parent's P-Chain height, increment | ||
// the epoch number, and set the epoch start time to the parent's timestamp. | ||
height, err := parent.pChainHeight(ctx) | ||
if err != nil { | ||
return 0, 0, time.Time{}, fmt.Errorf("failed to get P-Chain height: %w", err) | ||
} | ||
p.vm.ctx.Log.Info("parent sealed epoch. advancing epoch", | ||
zap.Uint64("height", height), | ||
zap.Uint64("epoch", epoch+1), | ||
) | ||
return height, epoch + 1, parentTimestamp, nil | ||
} | ||
// Otherwise, the parent did not seal the previous epoch, so the child should use the parent's | ||
// epoch information. This is true even if the child crosses the epoch boundary, since sealing | ||
// blocks are considered to be part of the epoch they seal. | ||
height, err := parent.pChainEpochHeight(ctx) | ||
if err != nil { | ||
return 0, 0, time.Time{}, fmt.Errorf("failed to get P-Chain height: %w", err) | ||
} | ||
p.vm.ctx.Log.Debug("parent did not seal epoch. using parent's epoch", | ||
zap.Uint64("height", height), | ||
zap.Uint64("epoch", epoch), | ||
) | ||
return height, epoch, epochStartTime, nil | ||
} | ||
|
||
// Verify returns nil if: | ||
// 1) [p]'s inner block is not an oracle block | ||
// 2) [child]'s P-Chain height >= [parentPChainHeight] | ||
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what about child has the expected epoch number and epoch start time? We need to verify every field we encode. |
||
func (p *postForkCommonComponents) Verify( | ||
ctx context.Context, | ||
parentTimestamp time.Time, | ||
|
@@ -163,9 +220,23 @@ func (p *postForkCommonComponents) Verify( | |
} | ||
|
||
var contextPChainHeight uint64 | ||
if p.vm.Upgrades.IsEtnaActivated(childTimestamp) { | ||
switch { | ||
case p.vm.Upgrades.IsGraniteActivated(childTimestamp): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what happens if the parent doesn't have granite activated but the child did? How should the system behave? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the |
||
pChainEpochHeight, _, _, err := p.getPChainEpoch(ctx, child.Parent()) | ||
if err != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We're checking the PchainEpochHeight is what it should be, what about the EpochNumber and EpochStartTime? shouldn't we check that too? |
||
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), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't we need to fail fast and return an error here? |
||
) | ||
} | ||
if childHeight := child.PChainEpochHeight(); pChainEpochHeight != childHeight { | ||
return fmt.Errorf("epoch height mismatch: expectedEpochHeight %d != epochHeight %d", pChainEpochHeight, childHeight) | ||
} | ||
contextPChainHeight = pChainEpochHeight | ||
case p.vm.Upgrades.IsEtnaActivated(childTimestamp): | ||
contextPChainHeight = childPChainHeight | ||
} else { | ||
default: | ||
contextPChainHeight = parentPChainHeight | ||
} | ||
|
||
|
@@ -225,10 +296,31 @@ func (p *postForkCommonComponents) buildChild( | |
return nil, err | ||
} | ||
|
||
var contextPChainHeight uint64 | ||
if p.vm.Upgrades.IsEtnaActivated(newTimestamp) { | ||
var ( | ||
contextPChainHeight, pChainEpochHeight, epochNumber uint64 | ||
epochStartTime time.Time | ||
) | ||
switch { | ||
case p.vm.Upgrades.IsGraniteActivated(newTimestamp): | ||
pChainEpochHeight, epochNumber, epochStartTime, err = p.getPChainEpoch(ctx, parentID) | ||
if err != nil { | ||
p.vm.ctx.Log.Error("unexpected build block failure", | ||
zap.String("reason", "failed to get P-Chain epoch height"), | ||
zap.Stringer("parentID", parentID), | ||
zap.Error(err), | ||
) | ||
} | ||
p.vm.ctx.Log.Debug( | ||
"epoch", | ||
zap.Uint64("pChainHeight", pChainHeight), | ||
zap.Uint64("pChainEpochHeight", pChainEpochHeight), | ||
zap.Uint64("epochNumber", epochNumber), | ||
zap.Time("epochStartTime", epochStartTime), | ||
) | ||
contextPChainHeight = pChainEpochHeight | ||
case p.vm.Upgrades.IsEtnaActivated(newTimestamp): | ||
contextPChainHeight = pChainHeight | ||
} else { | ||
default: | ||
contextPChainHeight = parentPChainHeight | ||
} | ||
|
||
|
@@ -251,6 +343,9 @@ func (p *postForkCommonComponents) buildChild( | |
parentID, | ||
newTimestamp, | ||
pChainHeight, | ||
pChainEpochHeight, | ||
epochNumber, | ||
epochStartTime, | ||
p.vm.StakingCertLeaf, | ||
innerBlock.Bytes(), | ||
p.vm.ctx.ChainID, | ||
|
@@ -261,6 +356,9 @@ func (p *postForkCommonComponents) buildChild( | |
parentID, | ||
newTimestamp, | ||
pChainHeight, | ||
pChainEpochHeight, | ||
epochNumber, | ||
epochStartTime, | ||
innerBlock.Bytes(), | ||
) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why? Can't we force its activation in the test?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can just do
upgrades.GraniteTime = upgrade.InitiallyActiveTime
and it will make it activated.