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.

Sign up for GitHub

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

Jump to bottom

fix(validation): added da batch validation against state BD desc #1218

Merged
merged 4 commits into from
Nov 12, 2024
Merged

fix(validation): added da batch validation against state BD desc #1218

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
54cbcc8
added da batch validation against state BD desc
mtsitrin Nov 11, 2024
fda6a5c
better validation when applying from SL
mtsitrin Nov 11, 2024
1ed5ee0
fix UT
mtsitrin Nov 11, 2024
1fb40f5
pr comments
mtsitrin Nov 12, 2024
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 block/manager_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -760,6 +760,7 @@ func TestDAFetch(t *testing.T) {
DA: c.daMetaData,
},
BlockDescriptors: bds,
EndHeight: batch.EndHeight(),
}
err := manager.ApplyBatchFromSL(slBatch)
require.Equal(c.err, err)
Expand Down
6 changes: 6 additions & 0 deletions block/retriever.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,12 @@ func (m *Manager) ApplyBatchFromSL(slBatch *settlement.Batch) error {
m.blockCache.Delete(block.Header.Height)
}
}

// validate the batch applied successfully and we are at the end height
Copy link
Contributor

Choose a reason for hiding this comment

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

if the batch isn't applied succesfully wouldn't we get an err from the applyBlockWithFraudHandling ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we're looping over for i, block := range batch.Blocks, which data on the DA
it can be empty/missing

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I got this @mtsitrin . Is it related to rollback on L1 truncating state info?

if m.State.Height() != slBatch.EndHeight {
danwt marked this conversation as resolved.
Show resolved Hide resolved
return fmt.Errorf("state height mismatch: state height: %d: batch end height: %d", m.State.Height(), slBatch.EndHeight)
danwt marked this conversation as resolved.
Show resolved Hide resolved
}

types.LastReceivedDAHeightGauge.Set(lastAppliedHeight)

return nil
Expand Down
65 changes: 28 additions & 37 deletions block/slvalidator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func (v *SettlementValidator) ValidateStateUpdate(batch *settlement.ResultRetrie
for height := batch.StartHeight; height <= batch.EndHeight; height++ {
source, err := v.blockManager.Store.LoadBlockSource(height)
if err != nil {
v.logger.Error("load block source", "error", err)
// v.logger.Error("load block source", "error", err)
Copy link
Contributor

Choose a reason for hiding this comment

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

i dont think you need to remove this log. this will just happen in case no block is stored for the height, which should not happen, i think, because there will be always a block applied for the validated height.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add the log back.
But I think it can happen
if u at height H, and the state info is [H-1, H+100],
u'll have 100 logs of it

Copy link
Contributor

@srene srene Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

im not sure this can happen. if you are at height h, and there is a new state update for h+100, then the node will sync first to height h+100, and then validate for the whole state info. upon reception of a new state update where endheight is higher than node height, it first calls the sync loop and from the sync loop it triggers validation once blocks are applied. so when a state info is validated the block should exist locally always, unless there is a bug.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes u right
it actually handled by the validation added to the ApplyBatchFromSL

mtsitrin marked this conversation as resolved.
Show resolved Hide resolved
continue
}

Expand All @@ -62,11 +62,9 @@ func (v *SettlementValidator) ValidateStateUpdate(batch *settlement.ResultRetrie
continue
}
p2pBlocks[block.Header.Height] = block

}

// load all DA blocks from the batch to be validated
daBlocks := []*types.Block{}
var daBatch da.ResultRetrieveBatch
for {
daBatch = v.blockManager.Retriever.RetrieveBatches(batch.MetaData.DA)
Expand All @@ -84,8 +82,12 @@ func (v *SettlementValidator) ValidateStateUpdate(batch *settlement.ResultRetrie
if errors.Is(checkBatchResult.Error, da.ErrBlobNotIncluded) {
return types.NewErrStateUpdateBlobNotAvailableFraud(batch.StateIndex, string(batch.MetaData.DA.Client), batch.MetaData.DA.Height, hex.EncodeToString(batch.MetaData.DA.Commitment))
}

// FIXME: how to handle non-happy case? not returning error?
continue
srene marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
continue

}

daBlocks := []*types.Block{}
for _, batch := range daBatch.Batches {
daBlocks = append(daBlocks, batch.Blocks...)
}
Expand Down Expand Up @@ -142,13 +144,9 @@ func (v *SettlementValidator) ValidateDaBlocks(slBatch *settlement.ResultRetriev
// we first verify the numblocks included in the state info match the block descriptors and the blocks obtained from DA
numSlBDs := uint64(len(slBatch.BlockDescriptors))
numSLBlocks := slBatch.NumBlocks
if numSLBlocks != numSlBDs {
return types.NewErrStateUpdateNumBlocksNotMatchingFraud(slBatch.EndHeight, numSLBlocks, numSLBlocks)
}

currentProposer := v.blockManager.State.GetProposer()
if currentProposer == nil {
return fmt.Errorf("proposer is not set")
numDABlocks := uint64(len(daBlocks))
if numSLBlocks != numSlBDs || numDABlocks < numSlBDs {
return types.NewErrStateUpdateNumBlocksNotMatchingFraud(slBatch.EndHeight, numSLBlocks, numSLBlocks, numDABlocks)
Comment on lines +148 to +149
Copy link
Contributor

Choose a reason for hiding this comment

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

why do we check numSLBlocks != numSlBDs . Isn't it guaranteed to be true by the hub?

}

// we compare all DA blocks against the information included in the state info block descriptors
Expand All @@ -172,36 +170,29 @@ func (v *SettlementValidator) ValidateDaBlocks(slBatch *settlement.ResultRetriev
if err != nil {
return err
}
}

// we compare the sequencer address between SL state info and DA block
// if next sequencer is not set, we check if the sequencer hash is equal to the next sequencer hash
// because it did not change. If the next sequencer is set, we check if the next sequencer hash is equal on the
// last block of the batch
isLastBlock := i == len(slBatch.BlockDescriptors)-1
if slBatch.NextSequencer != currentProposer.SettlementAddress && isLastBlock {
err := v.blockManager.UpdateSequencerSetFromSL()
if err != nil {
return fmt.Errorf("update sequencer set from SL: %w", err)
}
nextSequencer, found := v.blockManager.Sequencers.GetByAddress(slBatch.NextSequencer)
if !found {
return fmt.Errorf("next sequencer not found")
}
if !bytes.Equal(nextSequencer.MustHash(), daBlocks[i].Header.NextSequencersHash[:]) {
return types.NewErrInvalidNextSequencersHashFraud(
[32]byte(nextSequencer.MustHash()),
daBlocks[i].Header.NextSequencersHash,
)
}
} else {
if !bytes.Equal(daBlocks[i].Header.SequencerHash[:], daBlocks[i].Header.NextSequencersHash[:]) {
return types.NewErrInvalidNextSequencersHashFraud(
daBlocks[i].Header.SequencerHash,
daBlocks[i].Header.NextSequencersHash,
)
}
// we compare the sequencer address between SL state info and DA block
// if next sequencer is not set, we check if the sequencer hash is equal to the next sequencer hash
// because it did not change. If the next sequencer is set, we check if the next sequencer hash is equal on the
// last block of the batch
lastDABlock := daBlocks[len(daBlocks)-1]
mtsitrin marked this conversation as resolved.
Show resolved Hide resolved
mtsitrin marked this conversation as resolved.
Show resolved Hide resolved
expectedNextSeqHash := lastDABlock.Header.SequencerHash
if slBatch.NextSequencer != slBatch.Sequencer {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. instead of using state proposer, doing slBatch.NextSequencer != slBatch.Sequencer to check for a change
  2. skipped UpdateSequencerSetFromSL as we can assume sync with SL metadata

nextSequencer, found := v.blockManager.Sequencers.GetByAddress(slBatch.NextSequencer)
if !found {
return fmt.Errorf("next sequencer not found")
}
copy(expectedNextSeqHash[:], nextSequencer.MustHash())
}
// FIXME: support hard fork
if !bytes.Equal(expectedNextSeqHash[:], lastDABlock.Header.NextSequencersHash[:]) {
return types.NewErrInvalidNextSequencersHashFraud(
expectedNextSeqHash,
lastDABlock.Header.NextSequencersHash,
)
}

v.logger.Debug("DA blocks validated successfully", "start height", daBlocks[0].Header.Height, "end height", daBlocks[len(daBlocks)-1].Header.Height)
return nil
}
Expand Down
5 changes: 3 additions & 2 deletions block/slvalidator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ package block_test
import (
"crypto/rand"
"encoding/hex"
"github.com/tendermint/tendermint/crypto/ed25519"
tmtypes "github.com/tendermint/tendermint/types"
"reflect"
"testing"
"time"

"github.com/tendermint/tendermint/crypto/ed25519"
tmtypes "github.com/tendermint/tendermint/types"

"github.com/celestiaorg/celestia-openrpc/types/blob"
"github.com/dymensionxyz/dymint/block"
"github.com/dymensionxyz/dymint/da"
Expand Down
6 changes: 3 additions & 3 deletions block/validate.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,10 +30,10 @@ func (m *Manager) SettlementValidateLoop(ctx context.Context) error {
return ctx.Err()
case <-m.settlementValidationC:

m.logger.Info("validating state updates to target height", "targetHeight", min(m.LastSettlementHeight.Load(), m.State.Height()))

for currH := m.SettlementValidator.NextValidationHeight(); currH <= min(m.LastSettlementHeight.Load(), m.State.Height()); currH = m.SettlementValidator.NextValidationHeight() {
targetValidationHeight := min(m.LastSettlementHeight.Load(), m.State.Height())
m.logger.Info("validating state updates to target height", "targetHeight", targetValidationHeight)

for currH := m.SettlementValidator.NextValidationHeight(); currH <= targetValidationHeight; currH = m.SettlementValidator.NextValidationHeight() {
// get next batch that needs to be validated from SL
batch, err := m.SLClient.GetBatchAtHeight(currH)
if err != nil {
Expand Down
2 changes: 1 addition & 1 deletion settlement/settlement.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ type Batch struct {

// MetaData about the batch in the DA layer
MetaData *BatchMetaData
NumBlocks uint64
NumBlocks uint64 // FIXME: can be removed. not used and will be deprecated
}

type ResultRetrieveBatch struct {
Expand Down
4 changes: 4 additions & 0 deletions testutil/block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"github.com/dymensionxyz/dymint/indexers/txindex/kv"
"github.com/dymensionxyz/dymint/p2p"
"github.com/dymensionxyz/dymint/settlement"
"github.com/dymensionxyz/dymint/types"

"github.com/tendermint/tendermint/crypto/ed25519"
"github.com/tendermint/tendermint/libs/log"
Expand Down Expand Up @@ -132,6 +133,9 @@ func GetManagerWithProposerKey(conf config.BlockManagerConfig, proposerKey crypt
if err = p2pClient.Start(context.Background()); err != nil {
return nil, err
}

manager.Sequencers.Set([]types.Sequencer{*state.Proposer.Load()})

return manager, nil
}

Expand Down
1 change: 1 addition & 0 deletions testutil/types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,7 @@ func GenerateStateWithSequencer(initialHeight int64, lastBlockHeight int64, pubk
GenerateSettlementAddress(),
[]string{GenerateSettlementAddress()},
))

s.SetHeight(uint64(lastBlockHeight))
return s
}
Expand Down
6 changes: 4 additions & 2 deletions types/errors.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -325,18 +325,20 @@ type ErrStateUpdateNumBlocksNotMatchingFraud struct {
StateIndex uint64
SLNumBlocks uint64
NumBds uint64
NumDABlocks uint64
}

func NewErrStateUpdateNumBlocksNotMatchingFraud(stateIndex uint64, slNumBlocks uint64, numbds uint64) error {
func NewErrStateUpdateNumBlocksNotMatchingFraud(stateIndex, slNumBlocks, numbds, numDABlocks uint64) error {
return &ErrStateUpdateNumBlocksNotMatchingFraud{
StateIndex: stateIndex,
SLNumBlocks: slNumBlocks,
NumBds: numbds,
NumDABlocks: numDABlocks,
}
}

func (e ErrStateUpdateNumBlocksNotMatchingFraud) Error() string {
return fmt.Sprintf("numblocks not matching. StateIndex: %d Batch numblocks: %d Num of block descriptors: %d", e.StateIndex, e.SLNumBlocks, e.NumBds)
return fmt.Sprintf("blocks not matching. StateIndex: %d Batch numblocks: %d Num of block descriptors: %d Num of DA blocks: %d", e.StateIndex, e.SLNumBlocks, e.NumBds, e.NumDABlocks)
}

func (e ErrStateUpdateNumBlocksNotMatchingFraud) Unwrap() error {
Expand Down
Loading