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

fix(conductor): update for celestia-node v0.15.0 #1367

Merged
merged 1 commit into from
Aug 15, 2024

Conversation

ethanoroshiba
Copy link
Contributor

@ethanoroshiba ethanoroshiba commented Aug 14, 2024

Summary

Updated fetch_blobs_with_retry() to accept Celestia Node v0.15.0's new GetAll() return format. This will still work with Node v0.14.1 as well.

Background

celestia-node v0.15.0 now returns an empty list of blobs for GetAll if blobs are not found, which broke our current jsonrpsee error handling in conductor.

Changes

  • Updated fetch_blobs_with_retry() to accept the ParseError that Json RPC now throws when there are no blobs fetched.

Testing

E2E tests working

@github-actions github-actions bot added the conductor pertaining to the astria-conductor crate label Aug 14, 2024
@ethanoroshiba ethanoroshiba marked this pull request as ready for review August 14, 2024 16:49
@ethanoroshiba ethanoroshiba requested a review from a team as a code owner August 14, 2024 16:49
@ethanoroshiba ethanoroshiba added the celestia interacting with celestia label Aug 14, 2024
Copy link
Member

@SuperFluffy SuperFluffy left a comment

Choose a reason for hiding this comment

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

This checks if parsing failed and is extremely fragile.

Please try this branch instead: eigerco/lumina#340

@ethanoroshiba ethanoroshiba force-pushed the eoroshiba/celestia_upgrade_fix_conductor branch from 24d24c8 to cbae0c2 Compare August 14, 2024 18:31
@ethanoroshiba ethanoroshiba requested a review from a team as a code owner August 14, 2024 18:31
@github-actions github-actions bot added proto pertaining to the Astria Protobuf spec sequencer pertaining to the astria-sequencer crate sequencer-relayer pertaining to the astria-sequencer-relayer crate composer pertaining to composer cd labels Aug 14, 2024
@ethanoroshiba ethanoroshiba changed the base branch from quasystaty1/chore/celestia-local-upgrade to main August 14, 2024 18:31
@ethanoroshiba ethanoroshiba force-pushed the eoroshiba/celestia_upgrade_fix_conductor branch from cbae0c2 to 24865b5 Compare August 14, 2024 18:33
@ethanoroshiba ethanoroshiba removed proto pertaining to the Astria Protobuf spec sequencer pertaining to the astria-sequencer crate sequencer-relayer pertaining to the astria-sequencer-relayer crate composer pertaining to composer cd labels Aug 14, 2024
@ethanoroshiba ethanoroshiba removed request for a team and steezeburger August 14, 2024 19:06
@ethanoroshiba ethanoroshiba changed the title fix(conductor)!: update for celestia-node v0.15.0 fix(conductor): update for celestia-node v0.15.0 Aug 15, 2024
@joroshiba joroshiba dismissed SuperFluffy’s stale review August 15, 2024 17:31

discussed offline, important to get this in sooner than later. Can have better long term fix later.

@ethanoroshiba ethanoroshiba added this pull request to the merge queue Aug 15, 2024
Merged via the queue into main with commit 90d1cb3 Aug 15, 2024
51 checks passed
@ethanoroshiba ethanoroshiba deleted the eoroshiba/celestia_upgrade_fix_conductor branch August 15, 2024 19:21
steezeburger added a commit that referenced this pull request Aug 22, 2024
* main:
  refactor(core, proto)!: define app genesis state in proto (#1346)
  fix(sequencer): bump penumbra dep to fix ibc state access bug (#1389)
  feat(conductor)!: support disabled celestia auth (#1372)
  fix(sequencer)!: fix block fees (#1343)
  perf(sequencer): add benchmark for prepare_proposal (ENG-660) (#1337)
  fix(proto): fix import name mismatch (#1380)
  fix(ci): enable bridge withdrawer building with tag (#1374)
  feat(sequencer): rewrite memool to have per-account transaction storage and maintenance  (#1323)
  refactor(core, sequencer)!: require that bridge unlock address always be set (#1339)
  fix(sequencer)!: take funds from bridge in ics20 withdrawals (#1344)
  fix(sequencer)!: fix TOCTOU issues by merging check and execution (#1332)
  fix: abci error code (#1280)
  refactor(core): shorten `try_from_block_info_and_data()` (#1371)
  fix(relayer): change `reqwest` for `isahc` in relayer blackbox tests (ENG-699) (#1366)
  fix(conductor): update for celestia-node v0.15.0 (#1367)
  Chore: Upgrade celestia-node to v0.14.1 (#1360)
  chore(charts): fix charts production templates (#1359)
  chore(core, proto): migrate byte slices from Vec to Bytes (#1319)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
celestia interacting with celestia conductor pertaining to the astria-conductor crate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants