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.

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

vms/platformvm: Add decisionTxs parameter to NewBanffProposalBlock #2411

Merged
merged 28 commits into from
Dec 5, 2023
Merged
Show file tree
Hide file tree
Changes from 24 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
1fc80d6
`vms/platformvm`: Add `decisionTxs` parameter to `NewBanffProposalBlock`
dhrubabasu Dec 4, 2023
0c78641
add TODO
dhrubabasu Dec 4, 2023
e93ebf1
add test to `proposal_block_test.go`
dhrubabasu Dec 4, 2023
fd3f1ce
add parse test
dhrubabasu Dec 4, 2023
cee2b53
Merge branch 'dev' into start-supporting-decisiontxs-in-proposal-blks
dhrubabasu Dec 4, 2023
26711fa
cleanup
dhrubabasu Dec 4, 2023
63fdc95
fix
dhrubabasu Dec 4, 2023
a280929
nit
dhrubabasu Dec 4, 2023
144230a
update
dhrubabasu Dec 4, 2023
5fb3e41
Merge branch 'initialize-banff-decision-txs' into start-supporting-de…
dhrubabasu Dec 4, 2023
5695e2c
merged
dhrubabasu Dec 4, 2023
b3d7f4f
nit
dhrubabasu Dec 4, 2023
c17ba17
nit
dhrubabasu Dec 4, 2023
22934d9
fix
dhrubabasu Dec 4, 2023
b610af9
`vms/platformvm`: Cleanup some block tests
dhrubabasu Dec 4, 2023
2b151b2
nit
dhrubabasu Dec 4, 2023
a8741e6
Merge branch 'initialize-banff-decision-txs' into start-supporting-de…
dhrubabasu Dec 4, 2023
79e2b3c
Merge branch 'dev' into p-chain-block-test-cleanups
dhrubabasu Dec 4, 2023
e07feee
merged
dhrubabasu Dec 4, 2023
8d85762
nits
dhrubabasu Dec 4, 2023
b65af0b
Merge branch 'dev' into p-chain-block-test-cleanups
dhrubabasu Dec 4, 2023
3a004e3
Merge branch 'p-chain-block-test-cleanups' into start-supporting-deci…
dhrubabasu Dec 4, 2023
2e6741c
Merge branch 'dev' into p-chain-block-test-cleanups
Dec 5, 2023
bd0030d
Merge branch 'p-chain-block-test-cleanups' into start-supporting-deci…
dhrubabasu Dec 5, 2023
0fc575f
one test
dhrubabasu Dec 5, 2023
6e52b52
nit
dhrubabasu Dec 5, 2023
e29aef9
nits
dhrubabasu Dec 5, 2023
4c33ba4
merged
dhrubabasu Dec 5, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions vms/platformvm/block/builder/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ func buildBlock(
parentID,
height,
rewardValidatorTx,
[]*txs.Tx{}, // TODO: Populate with StandardBlock txs
)
}

Expand Down
1 change: 1 addition & 0 deletions vms/platformvm/block/builder/builder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -398,6 +398,7 @@ func TestBuildBlock(t *testing.T) {
parentID,
height,
tx,
[]*txs.Tx{},
)
require.NoError(err)
return expectedBlk
Expand Down
14 changes: 14 additions & 0 deletions vms/platformvm/block/executor/proposal_block_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -258,6 +258,7 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) {
parentID,
banffParentBlk.Height(),
blkTx,
[]*txs.Tx{},
)
require.NoError(err)

Expand Down Expand Up @@ -287,6 +288,7 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) {
parentID,
banffParentBlk.Height()+1,
blkTx,
[]*txs.Tx{},
)
require.NoError(err)

Expand All @@ -304,6 +306,7 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) {
parentID,
banffParentBlk.Height()+1,
blkTx,
[]*txs.Tx{},
)
require.NoError(err)

Expand All @@ -321,6 +324,7 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) {
parentID,
banffParentBlk.Height()+1,
blkTx,
[]*txs.Tx{},
)
require.NoError(err)

Expand All @@ -342,6 +346,7 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) {
parentID,
banffParentBlk.Height()+1,
invalidTx,
[]*txs.Tx{},
)
require.NoError(err)

Expand All @@ -357,6 +362,7 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) {
parentID,
banffParentBlk.Height()+1,
blkTx,
[]*txs.Tx{},
)
require.NoError(err)

Expand All @@ -373,6 +379,7 @@ func TestBanffProposalBlockTimeVerification(t *testing.T) {
parentID,
banffParentBlk.Height()+1,
blkTx,
[]*txs.Tx{},
)
require.NoError(err)

Expand Down Expand Up @@ -662,6 +669,7 @@ func TestBanffProposalBlockUpdateStakers(t *testing.T) {
parentBlk.ID(),
parentBlk.Height()+1,
s0RewardTx,
[]*txs.Tx{},
)
require.NoError(err)

Expand Down Expand Up @@ -819,6 +827,7 @@ func TestBanffProposalBlockRemoveSubnetValidator(t *testing.T) {
parentBlk.ID(),
parentBlk.Height()+1,
s0RewardTx,
[]*txs.Tx{},
)
require.NoError(err)
propBlk := env.blkManager.NewBlock(statelessProposalBlock)
Expand Down Expand Up @@ -931,6 +940,7 @@ func TestBanffProposalBlockTrackedSubnet(t *testing.T) {
parentBlk.ID(),
parentBlk.Height()+1,
s0RewardTx,
[]*txs.Tx{},
)
require.NoError(err)
propBlk := env.blkManager.NewBlock(statelessProposalBlock)
Expand Down Expand Up @@ -1017,6 +1027,7 @@ func TestBanffProposalBlockDelegatorStakerWeight(t *testing.T) {
parentBlk.ID(),
parentBlk.Height()+1,
s0RewardTx,
[]*txs.Tx{},
)
require.NoError(err)
propBlk := env.blkManager.NewBlock(statelessProposalBlock)
Expand Down Expand Up @@ -1108,6 +1119,7 @@ func TestBanffProposalBlockDelegatorStakerWeight(t *testing.T) {
parentBlk.ID(),
parentBlk.Height()+1,
s0RewardTx,
[]*txs.Tx{},
)
require.NoError(err)

Expand Down Expand Up @@ -1198,6 +1210,7 @@ func TestBanffProposalBlockDelegatorStakers(t *testing.T) {
parentBlk.ID(),
parentBlk.Height()+1,
s0RewardTx,
[]*txs.Tx{},
)
require.NoError(err)
propBlk := env.blkManager.NewBlock(statelessProposalBlock)
Expand Down Expand Up @@ -1288,6 +1301,7 @@ func TestBanffProposalBlockDelegatorStakers(t *testing.T) {
parentBlk.ID(),
parentBlk.Height()+1,
s0RewardTx,
[]*txs.Tx{},
)
require.NoError(err)
propBlk = env.blkManager.NewBlock(statelessProposalBlock)
Expand Down
1 change: 1 addition & 0 deletions vms/platformvm/block/executor/rejector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ func TestRejectBlock(t *testing.T) {
},
Creds: []verify.Verifiable{},
},
[]*txs.Tx{},
)
},
rejectFunc: func(r *rejector, b block.Block) error {
Expand Down
58 changes: 44 additions & 14 deletions vms/platformvm/block/parse_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,12 +25,12 @@ func TestStandardBlocks(t *testing.T) {
blkTimestamp := time.Now()
parentID := ids.ID{'p', 'a', 'r', 'e', 'n', 't', 'I', 'D'}
height := uint64(2022)
txs, err := testDecisionTxs()
decisionTxs, err := testDecisionTxs()
require.NoError(err)

for _, cdc := range []codec.Manager{Codec, GenesisCodec} {
// build block
apricotStandardBlk, err := NewApricotStandardBlock(parentID, height, txs)
apricotStandardBlk, err := NewApricotStandardBlock(parentID, height, decisionTxs)
require.NoError(err)

// parse block
Expand All @@ -44,10 +44,10 @@ func TestStandardBlocks(t *testing.T) {
require.Equal(apricotStandardBlk.Height(), parsed.Height())

require.IsType(&ApricotStandardBlock{}, parsed)
require.Equal(txs, parsed.Txs())
require.Equal(decisionTxs, parsed.Txs())

// check that banff standard block can be built and parsed
banffStandardBlk, err := NewBanffStandardBlock(blkTimestamp, parentID, height, txs)
banffStandardBlk, err := NewBanffStandardBlock(blkTimestamp, parentID, height, decisionTxs)
require.NoError(err)

// parse block
Expand All @@ -61,7 +61,7 @@ func TestStandardBlocks(t *testing.T) {
require.Equal(banffStandardBlk.Height(), parsed.Height())
require.IsType(&BanffStandardBlock{}, parsed)
parsedBanffStandardBlk := parsed.(*BanffStandardBlock)
require.Equal(txs, parsedBanffStandardBlk.Txs())
require.Equal(decisionTxs, parsedBanffStandardBlk.Txs())

// timestamp check for banff blocks only
require.Equal(banffStandardBlk.Timestamp(), parsedBanffStandardBlk.Timestamp())
Expand All @@ -77,15 +77,17 @@ func TestProposalBlocks(t *testing.T) {
blkTimestamp := time.Now()
parentID := ids.ID{'p', 'a', 'r', 'e', 'n', 't', 'I', 'D'}
height := uint64(2022)
tx, err := testProposalTx()
proposalTx, err := testProposalTx()
require.NoError(err)
decisionTxs, err := testDecisionTxs()
require.NoError(err)

for _, cdc := range []codec.Manager{Codec, GenesisCodec} {
// build block
apricotProposalBlk, err := NewApricotProposalBlock(
parentID,
height,
tx,
proposalTx,
)
require.NoError(err)

Expand All @@ -101,14 +103,15 @@ func TestProposalBlocks(t *testing.T) {

require.IsType(&ApricotProposalBlock{}, parsed)
parsedApricotProposalBlk := parsed.(*ApricotProposalBlock)
require.Equal([]*txs.Tx{tx}, parsedApricotProposalBlk.Txs())
require.Equal([]*txs.Tx{proposalTx}, parsedApricotProposalBlk.Txs())

// check that banff proposal block can be built and parsed
banffProposalBlk, err := NewBanffProposalBlock(
blkTimestamp,
parentID,
height,
tx,
proposalTx,
[]*txs.Tx{},
dhrubabasu marked this conversation as resolved.
Show resolved Hide resolved
)
require.NoError(err)

Expand All @@ -119,17 +122,44 @@ func TestProposalBlocks(t *testing.T) {
// compare content
require.Equal(banffProposalBlk.ID(), parsed.ID())
require.Equal(banffProposalBlk.Bytes(), parsed.Bytes())
require.Equal(banffProposalBlk.Parent(), banffProposalBlk.Parent())
require.Equal(banffProposalBlk.Parent(), parsed.Parent())
require.Equal(banffProposalBlk.Height(), parsed.Height())
require.IsType(&BanffProposalBlock{}, parsed)
parsedBanffProposalBlk := parsed.(*BanffProposalBlock)
require.Equal([]*txs.Tx{tx}, parsedBanffProposalBlk.Txs())
require.Equal([]*txs.Tx{proposalTx}, parsedBanffProposalBlk.Txs())

// timestamp check for banff blocks only
require.Equal(banffProposalBlk.Timestamp(), parsedBanffProposalBlk.Timestamp())

// backward compatibility check
require.Equal(parsedApricotProposalBlk.Txs(), parsedBanffProposalBlk.Txs())

// check that banff proposal block with decisionTxs can be built and parsed
banffProposalBlkWithDecisionTxs, err := NewBanffProposalBlock(
Copy link

Choose a reason for hiding this comment

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

Could we put decisionTxs in banffProposalBlk so that we don't need to build a new block here?

Copy link
Contributor Author

@dhrubabasu dhrubabasu Dec 5, 2023

Choose a reason for hiding this comment

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

I wanted to retain the backward compatibility check on line 135

// backward compatibility check
require.Equal(parsedApricotProposalBlk.Txs(), parsedBanffProposalBlk.Txs())

blkTimestamp,
parentID,
height,
proposalTx,
decisionTxs,
)
require.NoError(err)

// parse block
parsed, err = Parse(cdc, banffProposalBlkWithDecisionTxs.Bytes())
require.NoError(err)

// compare content
require.Equal(banffProposalBlkWithDecisionTxs.ID(), parsed.ID())
require.Equal(banffProposalBlkWithDecisionTxs.Bytes(), parsed.Bytes())
require.Equal(banffProposalBlkWithDecisionTxs.Parent(), parsed.Parent())
require.Equal(banffProposalBlkWithDecisionTxs.Height(), parsed.Height())
require.IsType(&BanffProposalBlock{}, parsed)
parsedBanffProposalBlkWithDecisionTxs := parsed.(*BanffProposalBlock)
expectedTxs := decisionTxs
expectedTxs = append(expectedTxs, proposalTx)
require.Equal(expectedTxs, parsedBanffProposalBlkWithDecisionTxs.Txs())

require.Equal(banffProposalBlkWithDecisionTxs.Timestamp(), parsedBanffProposalBlkWithDecisionTxs.Timestamp())
}
}

Expand Down Expand Up @@ -224,15 +254,15 @@ func TestAtomicBlock(t *testing.T) {
require := require.New(t)
parentID := ids.ID{'p', 'a', 'r', 'e', 'n', 't', 'I', 'D'}
height := uint64(2022)
tx, err := testAtomicTx()
atomicTx, err := testAtomicTx()
require.NoError(err)

for _, cdc := range []codec.Manager{Codec, GenesisCodec} {
// build block
atomicBlk, err := NewApricotAtomicBlock(
parentID,
height,
tx,
atomicTx,
)
require.NoError(err)

Expand All @@ -248,7 +278,7 @@ func TestAtomicBlock(t *testing.T) {

require.IsType(&ApricotAtomicBlock{}, parsed)
parsedAtomicBlk := parsed.(*ApricotAtomicBlock)
require.Equal([]*txs.Tx{tx}, parsedAtomicBlk.Txs())
require.Equal([]*txs.Tx{atomicTx}, parsedAtomicBlk.Txs())
}
}

Expand Down
8 changes: 5 additions & 3 deletions vms/platformvm/block/proposal_block.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,16 +67,18 @@ func NewBanffProposalBlock(
timestamp time.Time,
parentID ids.ID,
height uint64,
tx *txs.Tx,
proposalTx *txs.Tx,
decisionTxs []*txs.Tx,
) (*BanffProposalBlock, error) {
blk := &BanffProposalBlock{
Time: uint64(timestamp.Unix()),
Transactions: decisionTxs,
Time: uint64(timestamp.Unix()),
ApricotProposalBlock: ApricotProposalBlock{
CommonBlock: CommonBlock{
PrntID: parentID,
Hght: height,
},
Tx: tx,
Tx: proposalTx,
},
}
return blk, initialize(blk)
Expand Down
Loading
Loading