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

Remove stateful block from dsmr and move execute into node #1800

Merged
merged 4 commits into from
Nov 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
50 changes: 0 additions & 50 deletions x/dsmr/block.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,6 @@
package dsmr

import (
"context"
"time"

"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/utils/crypto/bls"
"github.com/ava-labs/avalanchego/utils/wrappers"
Expand Down Expand Up @@ -121,50 +118,3 @@ type Block struct {
func (b Block) GetID() ids.ID {
return b.blkID
}

// TODO: implement snowman.Block interface and integrate assembler
type StatefulBlock[T Tx, State any, B any, Result any] struct {
handler *BlockHandler[T, State, B, Result]
block *Block
}

func (s *StatefulBlock[T, S, B, R]) ID() ids.ID {
return s.block.blkID
}

func (s *StatefulBlock[T, S, B, R]) Parent() ids.ID {
return s.block.ParentID
}

func (s *StatefulBlock[T, S, B, R]) Verify(ctx context.Context) error {
// TODO: Verify header fields
// TODO: de-duplicate chunk certificates (internal to block and across history)
for _, chunkCert := range s.block.ChunkCerts {
// TODO: verify chunks within a provided context
if err := chunkCert.Verify(ctx, struct{}{}); err != nil {
return err
}
}

return nil
}

func (s *StatefulBlock[T, S, B, R]) Accept(ctx context.Context) error {
return s.handler.Accept(ctx, s.block)
}

func (*StatefulBlock[T, S, B, R]) Reject(context.Context) error {
return nil
}

func (s *StatefulBlock[T, S, B, R]) Bytes() []byte {
return s.block.blkBytes
}

func (s *StatefulBlock[T, S, B, R]) Height() uint64 {
return s.block.Height
}

func (s *StatefulBlock[T, S, B, R]) Timestamp() time.Time {
return time.Unix(0, s.block.Timestamp)
}
19 changes: 16 additions & 3 deletions x/dsmr/node.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,9 @@ type Node[T Tx] struct {
storage *chunkStorage[T]
}

// NewChunk builds transactions into a Chunk
// BuildChunk builds transactions into a Chunk
// TODO handle frozen sponsor + validator assignments
func (n *Node[T]) NewChunk(
func (n *Node[T]) BuildChunk(
ctx context.Context,
txs []T,
expiry int64,
Expand Down Expand Up @@ -171,7 +171,7 @@ func (n *Node[T]) NewChunk(
return chunk, n.storage.AddLocalChunkWithCert(chunk, &chunkCert)
}

func (n *Node[T]) NewBlock(parent Block, timestamp int64) (Block, error) {
func (n *Node[T]) BuildBlock(parent Block, timestamp int64) (Block, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If we're going to name this BuildBlock we should also rename NewChunk to BuildChunk to be consistent

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Will do

if timestamp <= parent.Timestamp {
return Block{}, ErrTimestampNotMonotonicallyIncreasing
}
Expand Down Expand Up @@ -207,6 +207,19 @@ func (n *Node[T]) NewBlock(parent Block, timestamp int64) (Block, error) {
return blk, nil
}

func (*Node[T]) Execute(ctx context.Context, _ Block, block Block) error {
Copy link
Contributor

@joshua-kim joshua-kim Nov 19, 2024

Choose a reason for hiding this comment

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

This was previously called Verify... Is this supposed to be Verify as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is meant to be renamed to Execute corresponding to #1744

Verify comes from AvalancheGo's snowman.Block interface and includes additional checks. The intent of Execute is that we are simply executing the state transition and assume that all extra information is supplied directly to the function ie. parent block, state, etc.

// TODO: Verify header fields
// TODO: de-duplicate chunk certificates (internal to block and across history)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Use general validity window to de-duplicate chunk certs

for _, chunkCert := range block.ChunkCerts {
// TODO: verify chunks within a provided context
if err := chunkCert.Verify(ctx, struct{}{}); err != nil {
return err
}
}

return nil
}

func (n *Node[T]) Accept(ctx context.Context, block Block) error {
chunkIDs := make([]ids.ID, 0, len(block.ChunkCerts))
for _, chunkCert := range block.ChunkCerts {
Expand Down
32 changes: 16 additions & 16 deletions x/dsmr/node_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ var (
)

// Test that chunks can be built through Node.NewChunk
func TestNode_NewChunk(t *testing.T) {
func TestNode_BuildChunk(t *testing.T) {
tests := []struct {
name string
txs []tx
Expand Down Expand Up @@ -119,7 +119,7 @@ func TestNode_NewChunk(t *testing.T) {
)
r.NoError(err)

chunk, err := node.NewChunk(
chunk, err := node.BuildChunk(
context.Background(),
tt.txs,
tt.expiry,
Expand Down Expand Up @@ -183,15 +183,15 @@ func TestNode_GetChunk_AvailableChunk(t *testing.T) {
)
r.NoError(err)

chunk, err := node.NewChunk(
chunk, err := node.BuildChunk(
context.Background(),
[]tx{{ID: ids.GenerateTestID(), Expiry: 123}},
123,
codec.Address{123},
)
r.NoError(err)

blk, err := node.NewBlock(
blk, err := node.BuildBlock(
Block{
ParentID: ids.GenerateTestID(),
Height: 0,
Expand Down Expand Up @@ -273,7 +273,7 @@ func TestNode_GetChunk_PendingChunk(t *testing.T) {
)
r.NoError(err)

chunk, err := node.NewChunk(
chunk, err := node.BuildChunk(
context.Background(),
[]tx{{ID: ids.GenerateTestID(), Expiry: 123}},
123,
Expand Down Expand Up @@ -566,7 +566,7 @@ func TestNode_BuiltChunksAvailableOverGetChunk(t *testing.T) {
// Build some chunks
wantChunks := make([]Chunk[tx], 0)
for _, args := range tt.availableChunks {
chunk, err := node.NewChunk(
chunk, err := node.BuildChunk(
context.Background(),
args.txs,
args.expiry,
Expand All @@ -577,7 +577,7 @@ func TestNode_BuiltChunksAvailableOverGetChunk(t *testing.T) {
wantChunks = append(wantChunks, chunk)
}

block, err := node.NewBlock(
block, err := node.BuildBlock(
Block{
ParentID: ids.GenerateTestID(),
Height: 0,
Expand Down Expand Up @@ -742,7 +742,7 @@ func TestNode_GetChunkSignature_SignValidChunk(t *testing.T) {
nil,
)
r.NoError(err)
chunk, err := node2.NewChunk(
chunk, err := node2.BuildChunk(
context.Background(),
[]tx{{ID: ids.Empty, Expiry: 123}},
123,
Expand Down Expand Up @@ -874,14 +874,14 @@ func TestNode_GetChunkSignature_DuplicateChunk(t *testing.T) {

// Accept a chunk
r.NoError(err)
chunk, err := node.NewChunk(
chunk, err := node.BuildChunk(
context.Background(),
[]tx{{ID: ids.Empty, Expiry: 123}},
123,
codec.Address{123},
)
r.NoError(err)
blk, err := node.NewBlock(
blk, err := node.BuildBlock(
Block{
ParentID: ids.GenerateTestID(),
Height: 0,
Expand Down Expand Up @@ -990,7 +990,7 @@ func TestGetChunkSignature_PersistAttestedBlocks(t *testing.T) {
)
r.NoError(err)

chunk, err := node2.NewChunk(
chunk, err := node2.BuildChunk(
context.Background(),
[]tx{{ID: ids.Empty, Expiry: 1}},
1,
Expand All @@ -1002,7 +1002,7 @@ func TestGetChunkSignature_PersistAttestedBlocks(t *testing.T) {
// chunk cert
var blk Block
for {
blk, err = node1.NewBlock(
blk, err = node1.BuildBlock(
Block{
ParentID: ids.Empty,
Height: 0,
Expand Down Expand Up @@ -1292,7 +1292,7 @@ func TestNode_NewBlock_IncludesChunkCerts(t *testing.T) {

wantChunks := make([]Chunk[tx], 0)
for _, chunk := range tt.chunks {
chunk, err := node.NewChunk(
chunk, err := node.BuildChunk(
context.Background(),
chunk.txs,
chunk.expiry,
Expand All @@ -1308,7 +1308,7 @@ func TestNode_NewBlock_IncludesChunkCerts(t *testing.T) {
wantChunks = append(wantChunks, chunk)
}

blk, err := node.NewBlock(tt.parent, tt.timestamp)
blk, err := node.BuildBlock(tt.parent, tt.timestamp)
r.ErrorIs(err, tt.wantErr)
if err != nil {
return
Expand Down Expand Up @@ -1376,14 +1376,14 @@ func TestAccept_RequestReferencedChunks(t *testing.T) {
)
r.NoError(err)

chunk, err := node1.NewChunk(
chunk, err := node1.BuildChunk(
context.Background(),
[]tx{{ID: ids.GenerateTestID(), Expiry: 1}},
1,
codec.Address{123},
)
r.NoError(err)
blk, err := node1.NewBlock(Block{
blk, err := node1.BuildBlock(Block{
ParentID: ids.GenerateTestID(),
Height: 0,
Timestamp: 0,
Expand Down
Loading