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

Add sequencer data to stream server #2517

Merged
merged 35 commits into from
Sep 19, 2023

Conversation

ToniRamirezM
Copy link
Contributor

@ToniRamirezM ToniRamirezM commented Sep 6, 2023

Closes #2515

What does this PR do?

Adds sequencer data to stream server.

New config params:

[Sequencer.StreamServer]
      Port = 6900
      Filename = "/datastreamer/datastream.bin"
      Enabled = true

Reviewers

Main reviewers:

@ToniRamirezM ToniRamirezM added this to the v0.3.1 milestone Sep 6, 2023
@ToniRamirezM ToniRamirezM self-assigned this Sep 6, 2023
@cla-bot cla-bot bot added the cla-signed label Sep 6, 2023
sequencer/datastream.go Outdated Show resolved Hide resolved
@ToniRamirezM ToniRamirezM changed the base branch from release/v0.3.1 to release/v0.3.2 September 8, 2023 10:19
@ToniRamirezM ToniRamirezM modified the milestones: v0.3.1, v0.3.2 Sep 8, 2023
@ARR552 ARR552 deleted the branch release/v0.3.2 September 8, 2023 13:20
@ARR552 ARR552 closed this Sep 8, 2023
@ToniRamirezM ToniRamirezM reopened this Sep 8, 2023
ARR552 and others added 2 commits September 8, 2023 16:03
* Fix rom ooc error detection

* remove check
sequencer/dbmanager.go Outdated Show resolved Hide resolved
sequencer/dbmanager.go Outdated Show resolved Hide resolved
sequencer/dbmanager.go Outdated Show resolved Hide resolved
* Use ZKEVM_NETWORK variable to specify location of config files in docker-compose

* fix config path in step 7.1
if err != nil {
return err
}

// Update batch l2 data
batch, err := d.state.GetBatchByNumber(ctx, tx.batchNumber, dbTx)
if err != nil {
err2 := dbTx.Rollback(ctx)
Copy link
Contributor

Choose a reason for hiding this comment

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

This rollback should not be done in line 182? as in the line 182 we are returning with an open db transaction (started in line 175)

if err != nil {
return err
// Send tx data to data stream server
if d.streamServer != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this code does the same:

if d.streamServer != nil {
		l2Block := DSL2Block{
			BatchNumber:    tx.batchNumber,
			L2BlockNumber:  l2BlochHeader.Number.Uint64(),
			Timestamp:      tx.timestamp,
			GlobalExitRoot: batch.GlobalExitRoot,
			Coinbase:       tx.coinbase,
		}

		l2Transaction := DSL2Transaction{
			BatchNumber:                 batch.BatchNumber,
			EffectiveGasPricePercentage: uint8(tx.response.EffectivePercentage),
			IsValid:                     1,
			EncodedLength:               uint32(len(txData)),
			Encoded:                     txData,
		}

		err = d.streamServer.StartAtomicOp()
		if err != nil {
			return err
		}

		_, err = d.streamServer.AddStreamEntry(EntryTypeL2Block, l2Block.Encode())
		if err != nil {
			return err
		}

		_, err = d.streamServer.AddStreamEntry(EntryTypeL2Tx, l2Transaction.Encode())
		if err != nil {
			return err
		}

		err = d.streamServer.CommitAtomicOp()
		if err != nil {
			log.Error("failed to commit atomic op: %v", err)
			err2 := dbTx.Rollback(ctx)
			if err2 != nil {
				log.Errorf("failed to rollback dbTx when committing atomic op that gave err: %v. Rollback err: %v", err2, err)
			}
			return err
		}
	} 

	err = dbTx.Commit(ctx)
	if err != nil {
		return err
	}

Copy link
Contributor Author

@ToniRamirezM ToniRamirezM Sep 14, 2023

Choose a reason for hiding this comment

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

If we do the error check this way, we would return without rolling back the stream transaction.

@@ -110,6 +110,10 @@ MaxTxLifetime = "3h"
ByteGasCost = 16
MarginFactor = 1
Enabled = false
[Sequencer.StreamServer]
Copy link
Contributor

Choose a reason for hiding this comment

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

StreamServer or DataStreamer? As we have used DataStreamerCfg for the config struct

@ToniRamirezM ToniRamirezM merged commit 34bfe58 into release/v0.3.2 Sep 19, 2023
@ToniRamirezM ToniRamirezM deleted the feature/sequencerDataStream branch September 19, 2023 11:17
@ToniRamirezM ToniRamirezM added the cherry-picked Content has been cherry-picked into a higher version branch label Sep 20, 2023
@ToniRamirezM ToniRamirezM mentioned this pull request Sep 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cherry-picked Content has been cherry-picked into a higher version branch cla-signed config
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants