Skip to content

Commit

Permalink
update
Browse files Browse the repository at this point in the history
  • Loading branch information
@tsachiherman
tsachiherman committed Nov 28, 2024
1 parent 6911f26 commit f2816e7
Show file tree
Hide file tree
Showing 3 changed files with 91 additions and 24 deletions.
10 changes: 10 additions & 0 deletions internal/validitywindow/validitywindow.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"fmt"
"sync"

"github.com/ava-labs/avalanchego/ids"
"github.com/ava-labs/avalanchego/trace"
"github.com/ava-labs/avalanchego/utils/logging"
"github.com/ava-labs/avalanchego/utils/set"
Expand Down Expand Up @@ -69,6 +70,15 @@ func (v *TimeValidityWindow[Container]) VerifyExpiryReplayProtection(
if dup.Len() > 0 {
return fmt.Errorf("%w: duplicate in ancestry", ErrDuplicateContainer)
}
// make sure we have no repeats within the block itself.
blkTxsIDs := make(map[ids.ID]bool, len(blk.Txs()))
for _, tx := range blk.Txs() {
id := tx.GetID()
if _, has := blkTxsIDs[id]; has {
return fmt.Errorf("%w: duplicate in block", ErrDuplicateContainer)
}
blkTxsIDs[id] = true
}
return nil
}

Expand Down
22 changes: 14 additions & 8 deletions x/dsmr/node.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ var (
ErrTimestampNotMonotonicallyIncreasing = errors.New("block timestamp must be greater than parent timestamp")
)

const validityWindowDuration = int64(5)

type (
ChainIndex = validitywindow.ChainIndex[*ChunkCertificate]
timeValidityWindow = *validitywindow.TimeValidityWindow[*ChunkCertificate]
Expand All @@ -60,6 +58,7 @@ func New[T Tx](
log logging.Logger,
tracer trace.Tracer,
chainIndex ChainIndex,
validityWindowDuration int64,
) (*Node[T], error) {
storage, err := newChunkStorage[T](NoVerifier[T]{}, memdb.New())
if err != nil {
Expand Down Expand Up @@ -93,10 +92,11 @@ func New[T Tx](
ChunkCertificateGossipHandler: &ChunkCertificateGossipHandler[T]{
storage: storage,
},
storage: storage,
log: log,
tracer: tracer,
validityWindow: validitywindow.NewTimeValidityWindow(log, tracer, chainIndex),
storage: storage,
log: log,
tracer: tracer,
validityWindow: validitywindow.NewTimeValidityWindow(log, tracer, chainIndex),
validityWindowDuration: validityWindowDuration,
}, nil
}

Expand All @@ -119,6 +119,7 @@ type (
storage *chunkStorage[T]
log logging.Logger
tracer trace.Tracer
validityWindowDuration int64
}
)

Expand Down Expand Up @@ -200,7 +201,7 @@ func (n *Node[T]) BuildBlock(ctx context.Context, parent Block, timestamp int64)
}

chunkCerts := n.storage.GatherChunkCerts()
oldestAllowed := timestamp - validityWindowDuration
oldestAllowed := timestamp - n.validityWindowDuration
if oldestAllowed < 0 {
oldestAllowed = 0
}
Expand Down Expand Up @@ -245,7 +246,12 @@ func (n *Node[T]) Execute(ctx context.Context, parentBlock Block, block Block) e
// TODO: Verify header fields

// Find repeats
if err := n.validityWindow.VerifyExpiryReplayProtection(ctx, block, parentBlock.Tmstmp); err != nil {

oldestAllowed := block.Timestamp() - n.validityWindowDuration
if oldestAllowed < 0 {
oldestAllowed = 0
}
if err := n.validityWindow.VerifyExpiryReplayProtection(ctx, block, oldestAllowed); err != nil {
return err
}

Expand Down
83 changes: 67 additions & 16 deletions x/dsmr/node_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,8 @@ var (
_ Verifier[tx] = (*failVerifier)(nil)
)

const testingDefaultValidityWindowDuration = int64(5)

type testingChainIndex struct {
blocks map[ids.ID]validitywindow.ExecutionBlock[*ChunkCertificate]
}
Expand Down Expand Up @@ -143,6 +145,7 @@ func TestNode_BuildChunk(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -210,6 +213,7 @@ func TestNode_GetChunk_AvailableChunk(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -304,6 +308,7 @@ func TestNode_GetChunk_PendingChunk(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -384,6 +389,7 @@ func TestNode_GetChunk_UnknownChunk(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -600,6 +606,7 @@ func TestNode_BuiltChunksAvailableOverGetChunk(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -736,6 +743,7 @@ func TestNode_GetChunkSignature_SignValidChunk(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -787,6 +795,7 @@ func TestNode_GetChunkSignature_SignValidChunk(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)
chunk, err := node2.BuildChunk(
Expand Down Expand Up @@ -908,6 +917,7 @@ func TestNode_GetChunkSignature_DuplicateChunk(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -1005,6 +1015,7 @@ func TestGetChunkSignature_PersistAttestedBlocks(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -1044,6 +1055,7 @@ func TestGetChunkSignature_PersistAttestedBlocks(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -1348,6 +1360,7 @@ func TestNode_NewBlock_IncludesChunkCerts(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -1437,6 +1450,7 @@ func TestDuplicateChunksElimination(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -1543,12 +1557,13 @@ func TestNode_Execute_Chunks(t *testing.T) {
return chunks
}
testCases := []struct {
name string
parentBlocks [][]int // for each parent, a list of the chunks included.
chunks []int
timestamp int64
executeWantErr error
buildWantErr error
name string
parentBlocks [][]int // for each parent, a list of the chunks included.
chunks []int
timestamp int64
executeWantErr error
buildWantErr error
validalidityWindowDuration int64
}{
{
name: "three empty blocks",
Expand All @@ -1566,22 +1581,43 @@ func TestNode_Execute_Chunks(t *testing.T) {
},
{
name: "two blocks one duplicate chunk",
parentBlocks: [][]int{{0, 1}, {1, 2}},
chunks: []int{},
timestamp: 4,
parentBlocks: [][]int{{0, 1}},
chunks: []int{1, 2},
timestamp: 2,
executeWantErr: validitywindow.ErrDuplicateContainer,
buildWantErr: nil, // build would filter out duplicate chunks, hence no error.
},
{
name: "one block duplicate chunks",
parentBlocks: [][]int{{1, 1}},
chunks: []int{},
timestamp: 4,
parentBlocks: [][]int{{}},
chunks: []int{1, 1},
timestamp: 2,
executeWantErr: validitywindow.ErrDuplicateContainer,
buildWantErr: nil, // build would filter out duplicate chunks, hence no error.
},
{
name: "three blocks non consecutive duplicate chunks",
parentBlocks: [][]int{{1}, {2}},
chunks: []int{1},
timestamp: 3,
executeWantErr: validitywindow.ErrDuplicateContainer,
},
{
name: "three blocks non consecutive duplicate chunks outside validity window",
parentBlocks: [][]int{{1}, {2}},
chunks: []int{1},
timestamp: 3,
executeWantErr: nil,
validalidityWindowDuration: 1,
},
}
for _, testCase := range testCases {
t.Run(testCase.name, func(t *testing.T) {
indexer := newTestingChainIndexer()
valWind := testCase.validalidityWindowDuration
if valWind == 0 {
valWind = testingDefaultValidityWindowDuration
}
node, err := New[tx](
ids.EmptyNodeID,
networkID,
Expand Down Expand Up @@ -1614,6 +1650,7 @@ func TestNode_Execute_Chunks(t *testing.T) {
logging.NoLog{},
trace.Noop,
indexer,
valWind,
)
r.NoError(err)

Expand All @@ -1631,9 +1668,9 @@ func TestNode_Execute_Chunks(t *testing.T) {
for _, chunkIndex := range chunkList {
blk.ChunkCerts = append(blk.ChunkCerts, makeChunkCert(chunks[chunkIndex]))
}
if blockNum > 0 {
r.ErrorIs(node.Execute(context.Background(), parentBlk, blk), testCase.executeWantErr)
}

r.NoError(node.Execute(context.Background(), parentBlk, blk))

r.NoError(node.Accept(context.Background(), blk))
indexer.set(blk.GetID(), blk)
parentBlk = blk
Expand All @@ -1642,8 +1679,20 @@ func TestNode_Execute_Chunks(t *testing.T) {
for _, chunkIdx := range testCase.chunks {
r.NoError(node.storage.AddLocalChunkWithCert(chunks[chunkIdx], makeChunkCert(chunks[chunkIdx])))
}
_, err = node.BuildBlock(context.Background(), parentBlk, testCase.timestamp)
newBlk, err := node.BuildBlock(context.Background(), parentBlk, testCase.timestamp)

Check failure on line 1682 in x/dsmr/node_test.go

View workflow job for this annotation

GitHub Actions / hypersdk-lint 

ineffectual assignment to newBlk (ineffassign)
r.ErrorIs(err, testCase.buildWantErr)

// create the block so that we can test it against the execute directly.
newBlk = Block{
ParentID: parentBlk.GetID(),
Hght: uint64(testCase.timestamp),
Tmstmp: int64(testCase.timestamp),

Check failure on line 1689 in x/dsmr/node_test.go

View workflow job for this annotation

GitHub Actions / hypersdk-lint 

unnecessary conversion (unconvert)
blkID: ids.GenerateTestID(),
}
for _, chunkIndex := range testCase.chunks {
newBlk.ChunkCerts = append(newBlk.ChunkCerts, makeChunkCert(chunks[chunkIndex]))
}
r.ErrorIs(node.Execute(context.Background(), parentBlk, newBlk), testCase.executeWantErr)
})
}
}
Expand Down Expand Up @@ -1691,6 +1740,7 @@ func TestAccept_RequestReferencedChunks(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)

Expand Down Expand Up @@ -1748,6 +1798,7 @@ func TestAccept_RequestReferencedChunks(t *testing.T) {
logging.NoLog{},
trace.Noop,
newTestingChainIndexer(),
testingDefaultValidityWindowDuration,
)
r.NoError(err)
r.NoError(node2.Accept(context.Background(), blk))
Expand Down

0 comments on commit f2816e7

Please sign in to comment.