Improve compression robustness - Part 1 (#1520)

* fix reorg issue - part 1

* revert wrong change

* fix gas

* fix

* regenerate bindings

* small fix

contracts/src/management/Structs.sol

@@ -4,11 +4,9 @@ pragma solidity >=0.7.0 <0.9.0;
 import * as MessageBusStructs from "../messaging/Structs.sol";
 
 interface Structs {
-     // MetaRollup is a rollup meta data
     struct MetaRollup{
         bytes32 Hash;
         address AggregatorID;
-        bytes32 L1Block;
         uint256 LastSequenceNumber;
     }
 
@@ -17,8 +15,6 @@ interface Structs {
     }
 
     struct HeaderCrossChainData {
-        uint256 blockNumber;
-        bytes32 blockHash;
         MessageBusStructs.Structs.CrossChainMessage[] messages;
     }
 }

go/common/headers.go

@@ -72,9 +72,8 @@ func (b *BatchHeader) MarshalJSON() ([]byte, error) {
 // RollupHeader is a public / plaintext struct that holds common properties of rollups.
 // All these fields are processed by the Management contract
 type RollupHeader struct {
-	L1Proof       L1BlockHash // the L1 block hash used by the enclave to generate the current rollup
-	L1ProofNumber *big.Int    // the height of the proof - used by the management contract to check
-	Coinbase      common.Address
+	Coinbase          common.Address
+	CompressionL1Head L1BlockHash // the l1 block that the sequencer considers canonical at the time when this rollup is created
 
 	CrossChainMessages []MessageBus.StructsCrossChainMessage `json:"crossChainMessages"`
 

go/common/rpc/converters.go

@@ -216,8 +216,7 @@ func ToRollupHeaderMsg(header *common.RollupHeader) *generated.RollupHeaderMsg {
 		return nil
 	}
 	headerMsg := generated.RollupHeaderMsg{
-		Proof:              header.L1Proof.Bytes(),
-		ProofNumber:        header.L1ProofNumber.Uint64(),
+		CompressionL1Head:  header.CompressionL1Head.Bytes(),
 		R:                  header.R.Bytes(),
 		S:                  header.S.Bytes(),
 		Coinbase:           header.Coinbase.Bytes(),
@@ -250,8 +249,7 @@ func FromRollupHeaderMsg(header *generated.RollupHeaderMsg) *common.RollupHeader
 	r := &big.Int{}
 	s := &big.Int{}
 	return &common.RollupHeader{
-		L1Proof:            gethcommon.BytesToHash(header.Proof),
-		L1ProofNumber:      big.NewInt(int64(header.ProofNumber)),
+		CompressionL1Head:  gethcommon.BytesToHash(header.CompressionL1Head),
 		R:                  r.SetBytes(header.R),
 		S:                  s.SetBytes(header.S),
 		Coinbase:           gethcommon.BytesToAddress(header.Coinbase),

go/common/rpc/generated/enclave.proto

@@ -396,7 +396,7 @@ message ExtRollupMsg {
 
 message RollupHeaderMsg {
   bytes ParentHash = 1;
-  bytes Proof = 2;
+  bytes CompressionL1Head = 2;
   uint64 ProofNumber = 3;
   uint64 Number = 4;
   uint64 Time = 5;

go/enclave/components/batch_registry.go

@@ -71,51 +71,62 @@ func (br *batchRegistry) HasGenesisBatch() (bool, error) {
 	return genesisBatchStored, nil
 }
 
-func (br *batchRegistry) BatchesAfter(batchSeqNo uint64, rollupLimiter limiters.RollupLimiter) ([]*core.Batch, error) {
-	batches := make([]*core.Batch, 0)
-
-	var batch *core.Batch
-	var err error
-	if batch, err = br.storage.FetchBatchBySeqNo(batchSeqNo); err != nil {
-		return nil, err
-	}
-	batches = append(batches, batch)
-
+func (br *batchRegistry) BatchesAfter(batchSeqNo uint64, upToL1Height uint64, rollupLimiter limiters.RollupLimiter) ([]*core.Batch, error) {
+	// sanity check
 	headBatch, err := br.storage.FetchHeadBatch()
 	if err != nil {
 		return nil, err
 	}
 
-	if headBatch.SeqNo().Uint64() < batch.SeqNo().Uint64() {
-		return nil, fmt.Errorf("head batch height %d is in the past compared to requested batch %d",
-			headBatch.SeqNo().Uint64(),
-			batch.SeqNo().Uint64())
+	if headBatch.SeqNo().Uint64() < batchSeqNo {
+		return nil, fmt.Errorf("head batch height %d is in the past compared to requested batch %d", headBatch.SeqNo().Uint64(), batchSeqNo)
 	}
-	for batch.SeqNo().Cmp(headBatch.SeqNo()) != 0 {
-		if didAcceptBatch, err := rollupLimiter.AcceptBatch(batch); err != nil {
-			return nil, err
-		} else if !didAcceptBatch {
-			return batches, nil
+
+	resultBatches := make([]*core.Batch, 0)
+
+	currentBatchSeq := batchSeqNo
+	for currentBatchSeq <= headBatch.SeqNo().Uint64() {
+		batch, err := br.storage.FetchBatchBySeqNo(currentBatchSeq)
+		if err != nil {
+			return nil, fmt.Errorf("could not retrieve batch by sequence number %d. Cause: %w", currentBatchSeq, err)
 		}
 
-		batch, err = br.storage.FetchBatchBySeqNo(batch.SeqNo().Uint64() + 1)
+		// check the block height
+		block, err := br.storage.FetchBlock(batch.Header.L1Proof)
 		if err != nil {
-			return nil, fmt.Errorf("could not retrieve batch by sequence number less than the head batch. Cause: %w", err)
+			return nil, fmt.Errorf("could not retrieve block. Cause: %w", err)
 		}
 
-		batches = append(batches, batch)
+		if block.NumberU64() > upToL1Height {
+			break
+		}
+
+		// check the limiter
+		didAcceptBatch, err := rollupLimiter.AcceptBatch(batch)
+		if err != nil {
+			return nil, err
+		}
+		if !didAcceptBatch {
+			break
+		}
+
+		resultBatches = append(resultBatches, batch)
 		br.logger.Info("Added batch to rollup", log.BatchHashKey, batch.Hash(), log.BatchSeqNoKey, batch.SeqNo(), log.BatchHeightKey, batch.Number(), "l1_proof", batch.Header.L1Proof)
+
+		currentBatchSeq++
 	}
 
-	// Sanity check that the rollup includes consecutive batches (according to the seqNo)
-	current := batches[0].SeqNo().Uint64()
-	for i, b := range batches {
-		if current+uint64(i) != b.SeqNo().Uint64() {
-			return nil, fmt.Errorf("created invalid rollup with batches out of sequence")
+	if len(resultBatches) > 0 {
+		// Sanity check that the rollup includes consecutive batches (according to the seqNo)
+		current := resultBatches[0].SeqNo().Uint64()
+		for i, b := range resultBatches {
+			if current+uint64(i) != b.SeqNo().Uint64() {
+				return nil, fmt.Errorf("created invalid rollup with batches out of sequence")
+			}
 		}
 	}
 
-	return batches, nil
+	return resultBatches, nil
 }
 
 func (br *batchRegistry) GetBatchStateAtHeight(blockNumber *gethrpc.BlockNumber) (*state.StateDB, error) {

go/enclave/components/interfaces.go

@@ -79,7 +79,7 @@ type BatchExecutor interface {
 
 type BatchRegistry interface {
 	// BatchesAfter - Given a hash, will return batches following it until the head batch
-	BatchesAfter(batchSeqNo uint64, rollupLimiter limiters.RollupLimiter) ([]*core.Batch, error)
+	BatchesAfter(batchSeqNo uint64, upToL1Height uint64, rollupLimiter limiters.RollupLimiter) ([]*core.Batch, error)
 
 	// GetBatchStateAtHeight - creates a stateDB that represents the state committed when
 	// the batch with height matching the blockNumber was created and stored.
@@ -103,7 +103,7 @@ type BatchRegistry interface {
 type RollupProducer interface {
 	// CreateRollup - creates a rollup starting from the end of the last rollup
 	// that has been stored and continues it towards what we consider the current L2 head.
-	CreateRollup(fromBatchNo uint64, limiter limiters.RollupLimiter) (*core.Rollup, error)
+	CreateRollup(fromBatchNo uint64, upToL1Height uint64, limiter limiters.RollupLimiter) (*core.Rollup, error)
 }
 
 type RollupConsumer interface {

go/enclave/components/rollup_compression.go

@@ -128,7 +128,7 @@ func (rc *RollupCompression) ProcessExtRollup(rollup *common.ExtRollup) error {
 	// The recreation of batches is a 2-step process:
 
 	// 1. calculate fields like: sequence, height, time, l1Proof, from the implicit and explicit information from the metadata
-	incompleteBatches, err := rc.createIncompleteBatches(calldataRollupHeader, transactionsPerBatch, rollup.Header.L1Proof)
+	incompleteBatches, err := rc.createIncompleteBatches(calldataRollupHeader, transactionsPerBatch, rollup.Header.CompressionL1Head)
 	if err != nil {
 		return err
 	}
@@ -249,15 +249,15 @@ func (rc *RollupCompression) createRollupHeader(batches []*core.Batch) (*common.
 }
 
 // the main logic to recreate the batches from the header. The logical pair of: `createRollupHeader`
-func (rc *RollupCompression) createIncompleteBatches(calldataRollupHeader *common.CalldataRollupHeader, transactionsPerBatch [][]*common.L2Tx, rollupL1Head common.L1BlockHash) ([]*batchFromRollup, error) {
+func (rc *RollupCompression) createIncompleteBatches(calldataRollupHeader *common.CalldataRollupHeader, transactionsPerBatch [][]*common.L2Tx, compressionL1Head common.L1BlockHash) ([]*batchFromRollup, error) {
 	incompleteBatches := make([]*batchFromRollup, len(transactionsPerBatch))
 
 	startAtSeq := calldataRollupHeader.FirstBatchSequence.Int64()
 	currentHeight := calldataRollupHeader.FirstCanonBatchHeight.Int64() - 1
 	currentTime := int64(calldataRollupHeader.StartTime)
 	var currentL1Height *big.Int
 
-	rollupL1Block, err := rc.storage.FetchBlock(rollupL1Head)
+	rollupL1Block, err := rc.storage.FetchBlock(compressionL1Head)
 	if err != nil {
 		return nil, err
 	}

go/enclave/components/rollup_producer.go

@@ -2,10 +2,11 @@ package components
 
 import (
 	"fmt"
+	"math/big"
 
-	"github.com/obscuronet/go-obscuro/go/enclave/storage"
+	"github.com/ethereum/go-ethereum/core/types"
 
-	"github.com/obscuronet/go-obscuro/go/common/log"
+	"github.com/obscuronet/go-obscuro/go/enclave/storage"
 
 	gethlog "github.com/ethereum/go-ethereum/log"
 
@@ -51,8 +52,8 @@ func NewRollupProducer(sequencerID gethcommon.Address, transactionBlobCrypto cry
 	}
 }
 
-func (re *rollupProducerImpl) CreateRollup(fromBatchNo uint64, limiter limiters.RollupLimiter) (*core.Rollup, error) {
-	batches, err := re.batchRegistry.BatchesAfter(fromBatchNo, limiter)
+func (re *rollupProducerImpl) CreateRollup(fromBatchNo uint64, upToL1Height uint64, limiter limiters.RollupLimiter) (*core.Rollup, error) {
+	batches, err := re.batchRegistry.BatchesAfter(fromBatchNo, upToL1Height, limiter)
 	if err != nil {
 		return nil, fmt.Errorf("could not fetch 'from' batch (seqNo=%d) for rollup: %w", fromBatchNo, err)
 	}
@@ -63,7 +64,11 @@ func (re *rollupProducerImpl) CreateRollup(fromBatchNo uint64, limiter limiters.
 		return nil, fmt.Errorf("no batches for rollup")
 	}
 
-	newRollup := re.createNextRollup(batches)
+	block, err := re.storage.FetchCanonicaBlockByHeight(big.NewInt(int64(upToL1Height)))
+	if err != nil {
+		return nil, err
+	}
+	newRollup := re.createNextRollup(batches, block)
 
 	re.logger.Info(fmt.Sprintf("Created new rollup %s with %d batches. From %d to %d", newRollup.Hash(), len(newRollup.Batches), batches[0].SeqNo(), batches[len(batches)-1].SeqNo()))
 
@@ -72,16 +77,11 @@ func (re *rollupProducerImpl) CreateRollup(fromBatchNo uint64, limiter limiters.
 
 // createNextRollup - based on a previous rollup and batches will create a new rollup that encapsulate the state
 // transition from the old rollup to the new one's head batch.
-func (re *rollupProducerImpl) createNextRollup(batches []*core.Batch) *core.Rollup {
+func (re *rollupProducerImpl) createNextRollup(batches []*core.Batch, block *types.Block) *core.Rollup {
 	lastBatch := batches[len(batches)-1]
 
 	rh := common.RollupHeader{}
-	rh.L1Proof = lastBatch.Header.L1Proof
-	b, err := re.storage.FetchBlock(rh.L1Proof)
-	if err != nil {
-		re.logger.Crit("Could not fetch block. Should not happen", log.ErrKey, err)
-	}
-	rh.L1ProofNumber = b.Number()
+	rh.CompressionL1Head = block.Hash()
 	rh.Coinbase = re.sequencerID
 
 	rh.CrossChainMessages = make([]MessageBus.StructsCrossChainMessage, 0)

go/enclave/enclave_test.go

@@ -38,7 +38,7 @@ const _testEnclavePublicKeyHex = "034d3b7e63a8bcd532ee3d1d6ecad9d67fca7821981a04
 // _successfulRollupGasPrice can be deterministically calculated when evaluating the management smart contract.
 // It should change only when there are changes to the smart contract or if the gas estimation algorithm is modified.
 // Other changes would mean something is broken.
-const _successfulRollupGasPrice = 319208
+const _successfulRollupGasPrice = 317480
 
 var _enclavePubKey *ecies.PublicKey
 
@@ -140,7 +140,7 @@ func gasEstimateSuccess(t *testing.T, w wallet.Wallet, enclave common.Enclave, v
 	}
 
 	if decodeUint64 != _successfulRollupGasPrice {
-		t.Fatal("unexpected gas price")
+		t.Fatalf("unexpected gas price %d", decodeUint64)
 	}
 }
 

go/enclave/nodetype/sequencer.go

@@ -30,6 +30,8 @@ import (
 	"github.com/obscuronet/go-obscuro/go/enclave/mempool"
 )
 
+const RollupDelay = 2 // number of L1 blocks to exclude when creating a rollup. This will minimize compression reorg issues.
+
 type SequencerSettings struct {
 	MaxBatchSize  uint64
 	MaxRollupSize uint64
@@ -239,10 +241,14 @@ func (s *sequencer) StoreExecutedBatch(batch *core.Batch, receipts types.Receipt
 }
 
 func (s *sequencer) CreateRollup(lastBatchNo uint64) (*common.ExtRollup, error) {
-	// todo @stefan - move this somewhere else, it shouldn't be in the batch registry.
 	rollupLimiter := limiters.NewRollupLimiter(s.settings.MaxRollupSize)
 
-	rollup, err := s.rollupProducer.CreateRollup(lastBatchNo, rollupLimiter)
+	currentL1Head, err := s.storage.FetchHeadBlock()
+	if err != nil {
+		return nil, err
+	}
+	upToL1Height := currentL1Head.NumberU64() - RollupDelay
+	rollup, err := s.rollupProducer.CreateRollup(lastBatchNo, upToL1Height, rollupLimiter)
 	if err != nil {
 		return nil, err
 	}

go/enclave/storage/enclavedb/block.go

@@ -154,7 +154,7 @@ func WriteRollup(dbtx DBTransaction, rollup *common.RollupHeader) error {
 		0,
 		0,
 		data,
-		rollup.L1Proof.Bytes(),
+		nil,
 	)
 	return nil
 }

go/ethadapter/mgmtcontractlib/mgmt_contract_lib.go

@@ -1,11 +1,8 @@
 package mgmtcontractlib
 
 import (
-	"bytes"
-	"compress/gzip"
 	"encoding/base64"
 	"fmt"
-	"io"
 	"math/big"
 	"strings"
 
@@ -82,11 +79,7 @@ func (c *contractLibImpl) DecodeTx(tx *types.Transaction) ethadapter.L1Transacti
 		if !found {
 			panic("call data not found for rollupData")
 		}
-		zipped := Base64DecodeFromString(callData.(string))
-		rollup, err := Decompress(zipped)
-		if err != nil {
-			panic(err)
-		}
+		rollup := Base64DecodeFromString(callData.(string))
 
 		return &ethadapter.L1RollupTx{
 			Rollup: rollup,
@@ -111,23 +104,16 @@ func (c *contractLibImpl) CreateRollup(t *ethadapter.L1RollupTx, nonce uint64) t
 		panic(err)
 	}
 
-	zipped, err := compress(t.Rollup)
-	if err != nil {
-		panic(err)
-	}
-	encRollupData := base64EncodeToString(zipped)
+	encRollupData := base64EncodeToString(t.Rollup)
 
 	metaRollup := ManagementContract.StructsMetaRollup{
 		Hash:               decodedRollup.Hash(),
 		AggregatorID:       decodedRollup.Header.Coinbase,
-		L1Block:            decodedRollup.Header.L1Proof,
 		LastSequenceNumber: big.NewInt(int64(decodedRollup.Header.LastBatchSeqNo)),
 	}
 
 	crossChain := ManagementContract.StructsHeaderCrossChainData{
-		BlockNumber: decodedRollup.Header.L1ProofNumber,
-		BlockHash:   decodedRollup.Header.L1Proof,
-		Messages:    convertCrossChainMessages(decodedRollup.Header.CrossChainMessages),
+		Messages: convertCrossChainMessages(decodedRollup.Header.CrossChainMessages),
 	}
 
 	data, err := c.contractABI.Pack(
@@ -320,19 +306,6 @@ func base64EncodeToString(bytes []byte) string {
 	return base64.StdEncoding.EncodeToString(bytes)
 }
 
-// compress the byte array using gzip
-func compress(in []byte) ([]byte, error) {
-	var b bytes.Buffer
-	gz := gzip.NewWriter(&b)
-	if _, err := gz.Write(in); err != nil {
-		return nil, err
-	}
-	if err := gz.Close(); err != nil {
-		return nil, err
-	}
-	return b.Bytes(), nil
-}
-
 // Base64DecodeFromString decodes a string to a byte array
 func Base64DecodeFromString(in string) []byte {
 	bytesStr, err := base64.StdEncoding.DecodeString(in)
@@ -342,18 +315,6 @@ func Base64DecodeFromString(in string) []byte {
 	return bytesStr
 }
 
-// Decompress the byte array using gzip
-func Decompress(in []byte) ([]byte, error) {
-	reader := bytes.NewReader(in)
-	gz, err := gzip.NewReader(reader)
-	if err != nil {
-		return nil, err
-	}
-	defer gz.Close()
-
-	return io.ReadAll(gz)
-}
-
 func convertCrossChainMessages(messages []MessageBus.StructsCrossChainMessage) []ManagementContract.StructsCrossChainMessage {
 	msgs := make([]ManagementContract.StructsCrossChainMessage, 0)