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

Writing to DA is blocking reading of new sequencer txs #122

Closed
2 tasks done
joroshiba opened this issue Jun 22, 2023 · 10 comments · Fixed by #150
Closed
2 tasks done

Writing to DA is blocking reading of new sequencer txs #122

joroshiba opened this issue Jun 22, 2023 · 10 comments · Fixed by #150
Assignees
Labels
bug Something isn't working celestia interacting with celestia demo sequencer-relayer pertaining to the astria-sequencer-relayer crate
Milestone

Comments

@joroshiba
Copy link
Member

joroshiba commented Jun 22, 2023

When I have sped up block times in our dev-cluster (see astriaorg/dev-cluster#35) the sequencer no longer consistently grabs blocks from metro. Looking at the log below, we can see timing wise that sequencer-relayer runs at the expected speed up until it starts seeing blocks. At that point there is significant delay in writing to celestia. We need to either make this non blocking, or bundle everything per celestia block into a single pfb.

I think there are two fixes we need here:

  1. We should be tracking sequencer block height and always be requesting the next block as opposed to the next head. Perhaps we grab all blocks between the head & last block
  2. Celestia batching & blocking of writing a PFB
2023-06-22T16:19:28.593842Z  WARN astria_sequencer_relayer::relayer: failed to get latest block from sequencer error=failed getting latest block

2023-06-22T16:19:29.593908Z  WARN astria_sequencer_relayer::relayer: failed to get latest block from sequencer error=failed getting latest block

2023-06-22T16:19:31.593417Z  WARN astria_sequencer_relayer::relayer: failed to get latest block from sequencer error=failed getting latest block

2023-06-22T16:19:32.593475Z  WARN astria_sequencer_relayer::relayer: failed to get latest block from sequencer error=failed getting latest block

2023-06-22T16:19:41.594699Z  INFO astria_sequencer_relayer::relayer: got block with height 3 from sequencer
2023-06-22T16:19:41.595136Z  WARN astria_sequencer_relayer::network: failed to publish block to network e=failed to publish message
2023-06-22T16:19:55.788947Z  INFO astria_sequencer_relayer::relayer: submitted sequencer block to DA layer sequencer_block=3 da_layer_block=37 tx_count=0

2023-06-22T16:19:55.790573Z  INFO astria_sequencer_relayer::relayer: got block with height 4 from sequencer
2023-06-22T16:19:55.790776Z  WARN astria_sequencer_relayer::network: failed to publish block to network e=failed to publish message
2023-06-22T16:20:20.797398Z  INFO astria_sequencer_relayer::relayer: submitted sequencer block to DA layer sequencer_block=4 da_layer_block=38 tx_count=0

2023-06-22T16:20:20.798946Z  INFO astria_sequencer_relayer::relayer: got block with height 8 from sequencer
2023-06-22T16:20:20.799397Z  WARN astria_sequencer_relayer::network: failed to publish block to network e=failed to publish message
2023-06-22T16:20:45.812921Z  INFO astria_sequencer_relayer::relayer: submitted sequencer block to DA layer sequencer_block=8 da_layer_block=39 tx_count=0

2023-06-22T16:20:45.815212Z  INFO astria_sequencer_relayer::relayer: got block with height 11 from sequencer
2023-06-22T16:20:45.815401Z  WARN astria_sequencer_relayer::network: failed to publish block to network e=failed to publish message

Tasks

Preview Give feedback
  1. celestia enhancement
    SuperFluffy
  2. celestia conductor sequencer-relayer
@joroshiba joroshiba added bug Something isn't working sequencer-relayer pertaining to the astria-sequencer-relayer crate demo labels Jun 22, 2023
@joroshiba joroshiba changed the title Failed gossip seems to create blocking operation resulting in missed sequencer blocks if block timing is fast Writing to DA blocks reading of new sequencer txs Jun 22, 2023
@noot noot self-assigned this Jun 22, 2023
@joroshiba
Copy link
Member Author

Currently sequencer-relayer has 1 block timing loop. Should we instead have 2 different loops? One for DA and one for sequencer.

On sequencer timing, read new blocks, gossip, add to DA queue. On DA timing write the queue to DA?

@noot
Copy link
Collaborator

noot commented Jun 22, 2023

there are multiple problems here:

  1. the relayer writing blocks on writing to celestia due to celestia's pay_for_data API being synchonous, ie. it only returns when the data is included in a celestia block, and thus this will block corresponding to celestia's block time. this can be fixed by putting the data write into its own tokio task, however this then opens us up to issue number 2:
  2. celestia does not allow us to submit multiple PFDs for the same namespace in one block; if you attempt this, it immediately rejects the PFDs that are not the first one and only includes the first. it doesn't even give us a good response, just that the response fields are empty and return immediately.

the only decent solution seems to be batching the writes on our end; so for each namespace we want to write to (sequencer, rollup), we batch all the writes into a Vec<SignedNamespaceData> and submit it once per celestia block time.

the conductor will need to be updated to handle a batched Vec<SignedNamespaceData> instead of just a SignedNamespaceData as it does now.

@joroshiba joroshiba added this to the MS-2 milestone Jun 23, 2023
@joroshiba joroshiba added the celestia interacting with celestia label Jun 23, 2023
@joroshiba joroshiba changed the title Writing to DA blocks reading of new sequencer txs Writing to DA is blocking reading of new sequencer txs Jun 26, 2023
@jbowen93
Copy link
Member

Might be worth looking at how Sovereign Labs has their Celestia adapter working: https://github.com/Sovereign-Labs/sovereign-sdk/tree/main/adapters/celestia

@joroshiba joroshiba assigned steezeburger and unassigned noot Jun 26, 2023
@noot
Copy link
Collaborator

noot commented Jun 27, 2023

@jbowen93 looks like their adaptor only does reading, not writing? :/

@renaynay
Copy link

Are you using the latest latest rc? rc5 @noot

@noot
Copy link
Collaborator

noot commented Jun 27, 2023

@renaynay I don't think so, looks like we've been using an older version: https://github.com/astriaorg/dev-cluster/blob/main/kubernetes/celestia-local/deployment.yml#L47

where can I find the API docs for rc5?

@renaynay
Copy link

@noot looks like ur using a custom image of node - can you tell me which version that points to?

@jbowen93
Copy link
Member

@noot
Copy link
Collaborator

noot commented Jun 27, 2023

@renaynay I believe were are using v0.6.1 of celestia-node and 0.11.0 of celestia-app. will be updating to use rc5, as the new Submit API here should unblock/fix this issue as we can submit multiple blobs in one tx https://node-rpc-docs.celestia.org/

@SuperFluffy
Copy link
Member

SuperFluffy commented Jul 11, 2023

There are two somewhat orthogonal issues here:

  1. architecturally sequencer-relayer and conductor are blocking their respective event loops when interacting with celestia; in general, in loop { select! { ... } } constructions the handler code should not be awaited, which both projects do. For example, sequencer relayer interacts with celestia using reqwest::Client or jsonrpsee::HttpClient and then awaits the response (which can take a while), blocking the loop from progressing.
  2. sequencer-relayer's CelestiaClient::{submit_namespaced_data, get_sequencer_block} issue one request per item; this further aggravates the previous architectural issues.

#150 tracked in #151 addresses the second point. But a proper fix is to resolve the architectural issues.

@SuperFluffy SuperFluffy linked a pull request Jul 12, 2023 that will close this issue
@joroshiba joroshiba modified the milestones: MS-2, MS-3 Jul 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working celestia interacting with celestia demo sequencer-relayer pertaining to the astria-sequencer-relayer crate
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants