fix(proto)!: Change execution API to use primitive RollupId by ethanoroshiba · Pull Request #1291 · astriaorg/astria

ethanoroshiba · 2024-07-23T18:31:39Z

Summary

Changed execution API rollup_id to use primitive RollupId instead of bytes.

Background

RollupId is defined in primitives protobuf specs, but astria.execution.v1alpha2.GenesisInfo.rollup_id was still using type bytes instead of the primitive type.

Changes

Changed execution rollup_id to be of type RollupId.
Regenerated Rust sources with protobuf compiler.
Changed affected conductor functions to utilize RollupId instead of bytes.
Updated evm-stack and evm-rollup versions.

Testing

With changes in astria-geth, passes smoke-test.

Breaking Changelist

Execution API changed, requires corresponding astria-geth changes to function properly.
Will require followup PR to update geth devTag to latest once geth PR is merged.

Related Issues

With astria-geth #38, closes #1287

aajimal · 2024-07-24T18:22:17Z

charts/evm-rollup/values.yaml

    repo: ghcr.io/astriaorg/astria-geth
    tag: 0.13.0
-    devTag: latest
+    devTag: pr-38


Just double-checking -- is this tag for all of development intentional?

Was intentional since this PR is dependent on astria-geth #38. @joroshiba suggested waiting until this PR merges, updating geth PR with latest protos and merging, then making a monorepo cleanup PR to update devTag to latest.

Yeah, because we have proto changes here that needs geth update, it gets a little weird here:

get this pr submitted which pushes prod proto changes

update pr-38 of geth to utilize new main proto libraries, update and submit.

update mono repo back to latest.

A bit of chicken/egg problem here. My preference is to keep astria-geth main from relying on non-main protos. We should wait to submit PR here until pr-38 is reviewed and approved though so we don't end up out of sync.

crates/astria-core/src/execution/v1alpha2/mod.rs

crates/astria-conductor/src/executor/tests.rs

crates/astria-conductor/src/executor/state.rs

crates/astria-core/src/primitive/v1/mod.rs

Co-authored-by: Jordan Oroshiba <jordan@astria.org>

crates/astria-conductor/tests/blackbox/helpers/macros.rs

Co-authored-by: Jordan Oroshiba <jordan@astria.org>

SuperFluffy

Looks great. Nothing to add to @joroshiba's review. Just a small nit in an error message.

crates/astria-core/src/execution/v1alpha2/mod.rs

SuperFluffy · 2024-07-25T19:35:37Z

crates/astria-core/src/execution/v1alpha2/mod.rs

            );
        Self::Raw {
-            rollup_id: Bytes::copy_from_slice(rollup_id.as_ref()),
+            rollup_id: Some(rollup_id.to_raw()),


Note that there is also into_raw. They do the same thing here, so it doesn't matter. But sometimes taking owernship allows some optimizations (like avoiding another allocation... if one remembers to implement it that way).

Got it! So for future reference if something is not a shared object then into_raw() may be a better choice?

I'll note that this is within a to_raw function. I have generally tried to keep the to_raw calling other to_raw and vice versa.

Co-authored-by: Richard Janis Goldschmidt <github@aberrat.io>

## Summary PR in tandem with astriaorg/astria#1291. Adjusts `GetGenesisInfo` to utilize primitive type `RollupId` instead of `bytes`. ## Changes - Updated grpc and protobuf SDKs. - Changed `GetGenesisInfo` to use primitive `RollupId`. - Updated sever tests to run correctly with `RollupId`.

## Summary Update EVM-Rollup Geth `devTag` after #1291 and astria-geth [#38](astriaorg/astria-geth#38). ## Background #1291 relied on astria-geth PR [#38](astriaorg/astria-geth#38) for `evm-rollup` chart. Now that both PRs have been merged, the `devTag` can be updated to `latest`. ## Changes - Updated geth `devTag` to `latest` in `evm-rollup` `values.yaml`. - Updated `evm-rollup` and `evm-stack` versions. ## Testing Passing e2e tests.

* main: release: cut bridge withdrawer release (#1303) release: version cuts for dusk-9 (#1299) chore(core): remove ed25519_consensus from public API (#1277) chore: remove spurious entry in gitignore (#1276) chore(chart): Update EVM-Rollup Geth devTag (#1300) fix(proto)!: Change execution API to use primitive RollupId (#1291) refactor(core, proto)!: define bridge memos in proto (#1285) chore(sequencer-relayer)!: minimize resubmissions to Celestia (#1234)

ethanoroshiba added 2 commits July 23, 2024 13:17

Change execution API to use primitive RollupId type

6091b7b

Merge branch 'main' into eoroshiba/exec_api_use_primitive

d0aae63

github-actions bot added conductor pertaining to the astria-conductor crate proto pertaining to the Astria Protobuf spec labels Jul 23, 2024

ethanoroshiba temporarily deployed to BUF July 23, 2024 18:32 — with GitHub Actions Inactive

rustfmt

a9ab197

ethanoroshiba temporarily deployed to BUF July 23, 2024 18:34 — with GitHub Actions Inactive

Edit geth devTag

73c4852

github-actions bot added the cd label Jul 23, 2024

ethanoroshiba temporarily deployed to BUF July 23, 2024 20:43 — with GitHub Actions Inactive

Change EVM chart version

91421dd

ethanoroshiba temporarily deployed to BUF July 23, 2024 21:01 — with GitHub Actions Inactive

ethanoroshiba mentioned this pull request Jul 23, 2024

Fix GetGenesisInfo after monorepo #1291 astriaorg/astria-geth#38

Merged

Fix EVM Geth devTag

a0b23c5

ethanoroshiba temporarily deployed to BUF July 23, 2024 21:21 — with GitHub Actions Inactive

Update versions

9f0e164

ethanoroshiba temporarily deployed to BUF July 24, 2024 13:58 — with GitHub Actions Inactive

Update stack version

cbfe459

ethanoroshiba temporarily deployed to BUF July 24, 2024 14:05 — with GitHub Actions Inactive

ethanoroshiba marked this pull request as ready for review July 24, 2024 14:51

ethanoroshiba requested review from a team as code owners July 24, 2024 14:51

ethanoroshiba requested review from aajimal and noot July 24, 2024 14:51

aajimal approved these changes Jul 24, 2024

View reviewed changes

aajimal reviewed Jul 24, 2024

View reviewed changes

joroshiba requested review from SuperFluffy and removed request for noot July 24, 2024 21:13

joroshiba requested changes Jul 25, 2024

View reviewed changes

Apply suggestions from code review

7190743

Co-authored-by: Jordan Oroshiba <jordan@astria.org>

ethanoroshiba temporarily deployed to BUF July 25, 2024 13:37 — with GitHub Actions Inactive

Merge branch 'main' into eoroshiba/exec_api_use_primitive

ec206b6

ethanoroshiba temporarily deployed to BUF July 25, 2024 14:35 — with GitHub Actions Inactive

minor updates

0b19858

ethanoroshiba temporarily deployed to BUF July 25, 2024 15:47 — with GitHub Actions Inactive

Declutter state test

294466f

ethanoroshiba temporarily deployed to BUF July 25, 2024 15:57 — with GitHub Actions Inactive

ethanoroshiba requested a review from joroshiba July 25, 2024 16:17

joroshiba reviewed Jul 25, 2024

View reviewed changes

crates/astria-conductor/tests/blackbox/helpers/macros.rs Outdated Show resolved Hide resolved

Commit suggestion

91af5b4

Co-authored-by: Jordan Oroshiba <jordan@astria.org>

ethanoroshiba temporarily deployed to BUF July 25, 2024 19:24 — with GitHub Actions Inactive

Merge branch 'main' into eoroshiba/exec_api_use_primitive

8b77110

ethanoroshiba temporarily deployed to BUF July 25, 2024 19:26 — with GitHub Actions Inactive

SuperFluffy approved these changes Jul 25, 2024

View reviewed changes

Apply Janis

4e70f49

Co-authored-by: Richard Janis Goldschmidt <github@aberrat.io>

ethanoroshiba temporarily deployed to BUF July 25, 2024 19:43 — with GitHub Actions Inactive

joroshiba approved these changes Jul 25, 2024

View reviewed changes

ethanoroshiba added this pull request to the merge queue Jul 25, 2024

Merged via the queue into main with commit 4b014f7 Jul 25, 2024

ethanoroshiba deleted the eoroshiba/exec_api_use_primitive branch July 25, 2024 20:53

ethanoroshiba mentioned this pull request Jul 25, 2024

chore(chart): Update EVM-Rollup Geth devTag #1300

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(proto)!: Change execution API to use primitive RollupId#1291

fix(proto)!: Change execution API to use primitive RollupId#1291
ethanoroshiba merged 15 commits intomainfrom
eoroshiba/exec_api_use_primitive

ethanoroshiba commented Jul 23, 2024 •

edited

Loading

Uh oh!

aajimal Jul 24, 2024

Uh oh!

ethanoroshiba Jul 24, 2024

Uh oh!

joroshiba Jul 24, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SuperFluffy left a comment •

edited

Loading

Uh oh!

Uh oh!

SuperFluffy Jul 25, 2024

Uh oh!

ethanoroshiba Jul 25, 2024

Uh oh!

joroshiba Jul 25, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

ethanoroshiba commented Jul 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Background

Changes

Testing

Breaking Changelist

Related Issues

Uh oh!

aajimal Jul 24, 2024

Choose a reason for hiding this comment

Uh oh!

ethanoroshiba Jul 24, 2024

Choose a reason for hiding this comment

Uh oh!

joroshiba Jul 24, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

SuperFluffy left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SuperFluffy Jul 25, 2024

Choose a reason for hiding this comment

Uh oh!

ethanoroshiba Jul 25, 2024

Choose a reason for hiding this comment

Uh oh!

joroshiba Jul 25, 2024

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

ethanoroshiba commented Jul 23, 2024 •

edited

Loading

SuperFluffy left a comment •

edited

Loading