Skip to content

Commit

Permalink
fix(ci, core)!: ensure committed and code generated from protobuf spe…
Browse files Browse the repository at this point in the history
…c match (#1825)

## Summary
Deletes and regenerates all code in `astria-core/src/generated`. Fixes
bad imports by nesting generated Astria APIs under a new `astria`
module. Fixes badly named modules.

## Background
#1236 noticed that some protobuf
files were skipped during code generation because of a
`tonic_build::Builder::extern_path` statement. This showed up again when
reworking the protobuf compilation tool to first clear the target
directory for storing the generated rust files, where now an `include!`
statement in `astria_core::generated` failed because the file was not in
fact generated. This was not detected because the file was still
committed to the repository at an earlier point.

#1707 then added new generated
code under a wrongly named `optimistic_block` module, which should have
been named `optimistic` to be in line with its protobuf package
counterpart.

The present change is primarily to the `tools/protobuf-compiler` binary,
which first purges the output directory (minus the handwritten `mod.rs`)
before generating new code.

## Changes
- Update `tools/protobuf-compiler` to clear `astria-core/src/generated`
prior to repopulating it from `proto/`
- Remove the module `sequencerblock::optimisticblock`
- Add module `sequencerblock::optimistic`
- Update all services that import Astria APIs from
`astria_core::generated` to `astria_core::generated::astria`.

## Testing
This is just code organization. Import paths were updated, code still
compiles and tests pass.

## Changelogs
Changelogs updated.

## Breaking Changelist
- This is a breaking change for all consumers of `astria_core` that now
need to import many items from `astria_core::generated::astria` that
were directly under `astria_core::generated` before.

## Override Freeze
This code touches services by changing their imports
(`astria_core::generated -> astria_core::generated::astria`) but does
change any implementation details.

## Related Issues
Fixes and amends #1707.
Supersedes #1824
Closes #1823
  • Loading branch information
SuperFluffy authored Dec 5, 2024
1 parent e1a4f02 commit fc10a63
Show file tree
Hide file tree
Showing 71 changed files with 295 additions and 1,221 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
time::Duration,
};

use astria_core::generated::sequencerblock::v1::sequencer_service_client::SequencerServiceClient;
use astria_core::generated::astria::sequencerblock::v1::sequencer_service_client::SequencerServiceClient;
use astria_eyre::eyre::{
self,
WrapErr as _,
Expand Down
19 changes: 10 additions & 9 deletions crates/astria-bridge-withdrawer/src/bridge_withdrawer/startup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
};

use astria_core::{
generated::sequencerblock::v1::sequencer_service_client::{
generated::astria::sequencerblock::v1::sequencer_service_client::{
self,
SequencerServiceClient,
},
Expand Down Expand Up @@ -270,20 +270,21 @@ impl Startup {
the sequencer logic."
);

let proto_tx = astria_core::generated::protocol::transaction::v1::Transaction::decode(
&*last_transaction.tx,
)
.wrap_err_with(|| {
format!(
let proto_tx =
astria_core::generated::astria::protocol::transaction::v1::Transaction::decode(
&*last_transaction.tx,
)
.wrap_err_with(|| {
format!(
"failed to decode data in Sequencer CometBFT transaction as `{}`",
astria_core::generated::protocol::transaction::v1::Transaction::full_name(),
astria_core::generated::astria::protocol::transaction::v1::Transaction::full_name(),
)
})?;
})?;

let tx = Transaction::try_from_raw(proto_tx).wrap_err_with(|| {
format!(
"failed to verify {}",
astria_core::generated::protocol::transaction::v1::Transaction::full_name()
astria_core::generated::astria::protocol::transaction::v1::Transaction::full_name()
)
})?;

Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::sync::Arc;

use astria_core::generated::sequencerblock::v1::sequencer_service_client::SequencerServiceClient;
use astria_core::generated::astria::sequencerblock::v1::sequencer_service_client::SequencerServiceClient;
use astria_eyre::eyre::{
self,
Context as _,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use std::{
};

use astria_core::{
generated::sequencerblock::v1::{
generated::astria::sequencerblock::v1::{
sequencer_service_client::{
self,
SequencerServiceClient,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ fn prepare_broadcast_tx_sync_response(response: tx_sync::Response) -> Mock {

/// Convert a wiremock request to an astria transaction
pub fn tx_from_request(request: &wiremock::Request) -> Transaction {
use astria_core::generated::protocol::transaction::v1::Transaction as RawTransaction;
use astria_core::generated::astria::protocol::transaction::v1::Transaction as RawTransaction;
use prost::Message as _;

let wrapped_tx_sync_req: tendermint_rpc::request::Wrapper<tx_sync::Request> =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{

use astria_core::{
self,
generated::sequencerblock::v1::{
generated::astria::sequencerblock::v1::{
sequencer_service_server::{
SequencerService,
SequencerServiceServer,
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-cli/src/sequencer/threshold/sign.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::collections::BTreeMap;

use astria_core::generated::protocol::transaction::v1::{
use astria_core::generated::astria::protocol::transaction::v1::{
Transaction,
TransactionBody,
};
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-cli/src/sequencer/threshold/verify.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use astria_core::generated::protocol::transaction::v1::TransactionBody;
use astria_core::generated::astria::protocol::transaction::v1::TransactionBody;
use color_eyre::eyre::{
self,
WrapErr as _,
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-composer/src/collectors/grpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use std::sync::Arc;

use astria_core::{
generated::composer::v1::{
generated::astria::composer::v1::{
grpc_collector_service_server::GrpcCollectorService,
SubmitRollupTransactionRequest,
SubmitRollupTransactionResponse,
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-composer/src/executor/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{

use astria_core::{
crypto::SigningKey,
generated::sequencerblock::v1::sequencer_service_client::SequencerServiceClient,
generated::astria::sequencerblock::v1::sequencer_service_client::SequencerServiceClient,
primitive::v1::Address,
protocol::transaction::v1::action::RollupDataSubmission,
};
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-composer/src/executor/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use std::{

use astria_core::{
crypto::SigningKey,
generated::sequencerblock::v1::{
generated::astria::sequencerblock::v1::{
sequencer_service_client::{
self,
SequencerServiceClient,
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-composer/src/grpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use std::net::SocketAddr;

use astria_core::{
generated::composer::v1::grpc_collector_service_server::GrpcCollectorServiceServer,
generated::astria::composer::v1::grpc_collector_service_server::GrpcCollectorServiceServer,
primitive::v1::asset,
};
use astria_eyre::{
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-composer/tests/blackbox/grpc_collector.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use std::time::Duration;

use astria_core::{
generated::composer::v1::{
generated::astria::composer::v1::{
grpc_collector_service_client::GrpcCollectorServiceClient,
SubmitRollupTransactionRequest,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use std::{

use astria_core::{
self,
generated::sequencerblock::v1::{
generated::astria::sequencerblock::v1::{
sequencer_service_server::{
SequencerService,
SequencerServiceServer,
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-composer/tests/blackbox/helper/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ pub async fn loop_until_composer_is_ready(addr: SocketAddr) {
}

fn signed_tx_from_request(request: &Request) -> Transaction {
use astria_core::generated::protocol::transaction::v1::Transaction as RawTransaction;
use astria_core::generated::astria::protocol::transaction::v1::Transaction as RawTransaction;
use prost::Message as _;

let wrapped_tx_sync_req: request::Wrapper<tx_sync::Request> =
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-conductor/src/celestia/block_verifier.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ mod tests {
use std::collections::BTreeMap;

use astria_core::{
generated::sequencerblock::v1::SequencerBlockHeader as RawSequencerBlockHeader,
generated::astria::sequencerblock::v1::SequencerBlockHeader as RawSequencerBlockHeader,
primitive::v1::RollupId,
sequencerblock::v1::{
block::SequencerBlockHeader,
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-conductor/src/celestia/convert.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use astria_core::{
brotli::decompress_bytes,
generated::sequencerblock::v1::{
generated::astria::sequencerblock::v1::{
SubmittedMetadataList,
SubmittedRollupDataList,
},
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-conductor/src/executor/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use astria_core::{
CommitmentState,
GenesisInfo,
},
generated::{
generated::astria::{
execution::{
v1 as raw,
v1::execution_service_client::ExecutionServiceClient,
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-conductor/src/executor/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ pub(super) fn map_sequencer_height_to_rollup_height(
#[cfg(test)]
mod tests {
use astria_core::{
generated::execution::v1 as raw,
generated::astria::execution::v1 as raw,
Protobuf as _,
};
use pbjson_types::Timestamp;
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-conductor/src/executor/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use astria_core::{
CommitmentState,
GenesisInfo,
},
generated::execution::v1 as raw,
generated::astria::execution::v1 as raw,
Protobuf as _,
};
use bytes::Bytes;
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-conductor/src/sequencer/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
use std::time::Duration;

use astria_core::{
generated::sequencerblock::v1::{
generated::astria::sequencerblock::v1::{
sequencer_service_client::SequencerServiceClient,
GetFilteredSequencerBlockRequest,
},
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-conductor/tests/blackbox/firm_only.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use astria_conductor::{
Conductor,
Config,
};
use astria_core::generated::execution::v1::{
use astria_core::generated::astria::execution::v1::{
GetCommitmentStateRequest,
GetGenesisInfoRequest,
};
Expand Down
14 changes: 7 additions & 7 deletions crates/astria-conductor/tests/blackbox/helpers/macros.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#[macro_export]
macro_rules! block {
(number: $number:expr,hash: $hash:expr,parent: $parent:expr $(,)?) => {
::astria_core::generated::execution::v1::Block {
::astria_core::generated::astria::execution::v1::Block {
number: $number,
hash: ::bytes::Bytes::from(Vec::from($hash)),
parent_block_hash: ::bytes::Bytes::from(Vec::from($parent)),
Expand Down Expand Up @@ -59,7 +59,7 @@ macro_rules! commitment_state {
soft: (number: $soft_number:expr,hash: $soft_hash:expr,parent: $soft_parent:expr $(,)?),
base_celestia_height: $base_celestia_height:expr $(,)?
) => {
::astria_core::generated::execution::v1::CommitmentState {
::astria_core::generated::astria::execution::v1::CommitmentState {
firm: Some($crate::block!(
number: $firm_number,
hash: $firm_hash,
Expand Down Expand Up @@ -98,7 +98,7 @@ macro_rules! genesis_info {
$sequencer_height:expr,celestia_block_variance:
$variance:expr $(,)?
) => {
::astria_core::generated::execution::v1::GenesisInfo {
::astria_core::generated::astria::execution::v1::GenesisInfo {
rollup_id: Some($crate::ROLLUP_ID.to_raw()),
sequencer_genesis_block_height: $sequencer_height,
celestia_block_variance: $variance,
Expand Down Expand Up @@ -312,7 +312,7 @@ macro_rules! mount_get_filtered_sequencer_block {
($test_env:ident, sequencer_height: $height:expr, delay: $delay:expr $(,)?) => {
$test_env
.mount_get_filtered_sequencer_block(
::astria_core::generated::sequencerblock::v1::GetFilteredSequencerBlockRequest {
::astria_core::generated::astria::sequencerblock::v1::GetFilteredSequencerBlockRequest {
height: $height,
rollup_ids: vec![$crate::ROLLUP_ID.to_raw()],
},
Expand Down Expand Up @@ -385,12 +385,12 @@ macro_rules! mount_get_block {
hash: $hash,
parent: $parent,
);
let identifier = ::astria_core::generated::execution::v1::BlockIdentifier {
let identifier = ::astria_core::generated::astria::execution::v1::BlockIdentifier {
identifier: Some(
::astria_core::generated::execution::v1::block_identifier::Identifier::BlockNumber(block.number)
::astria_core::generated::astria::execution::v1::block_identifier::Identifier::BlockNumber(block.number)
)};
$test_env.mount_get_block(
::astria_core::generated::execution::v1::GetBlockRequest {
::astria_core::generated::astria::execution::v1::GetBlockRequest {
identifier: Some(identifier),
},
block,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{
sync::Arc,
};

use astria_core::generated::{
use astria_core::generated::astria::{
execution::v1::{
execution_service_server::{
ExecutionService,
Expand Down
12 changes: 6 additions & 6 deletions crates/astria-conductor/tests/blackbox/helpers/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use astria_conductor::{
};
use astria_core::{
brotli::compress_bytes,
generated::{
generated::astria::{
execution::v1::{
Block,
CommitmentState,
Expand Down Expand Up @@ -179,7 +179,7 @@ impl TestConductor {
pub async fn mount_get_block<S: serde::Serialize>(
&self,
expected_pbjson: S,
block: astria_core::generated::execution::v1::Block,
block: astria_core::generated::astria::execution::v1::Block,
) {
use astria_grpc_mock::{
matcher::message_partial_pbjson,
Expand Down Expand Up @@ -306,7 +306,7 @@ impl TestConductor {
}

pub async fn mount_get_genesis_info(&self, genesis_info: GenesisInfo) {
use astria_core::generated::execution::v1::GetGenesisInfoRequest;
use astria_core::generated::astria::execution::v1::GetGenesisInfoRequest;
astria_grpc_mock::Mock::for_rpc_given(
"get_genesis_info",
astria_grpc_mock::matcher::message_type::<GetGenesisInfoRequest>(),
Expand All @@ -318,7 +318,7 @@ impl TestConductor {
}

pub async fn mount_get_commitment_state(&self, commitment_state: CommitmentState) {
use astria_core::generated::execution::v1::GetCommitmentStateRequest;
use astria_core::generated::astria::execution::v1::GetCommitmentStateRequest;

astria_grpc_mock::Mock::for_rpc_given(
"get_commitment_state",
Expand Down Expand Up @@ -381,7 +381,7 @@ impl TestConductor {
commitment_state: CommitmentState,
expected_calls: u64,
) -> astria_grpc_mock::MockGuard {
use astria_core::generated::execution::v1::UpdateCommitmentStateRequest;
use astria_core::generated::astria::execution::v1::UpdateCommitmentStateRequest;
use astria_grpc_mock::{
matcher::message_partial_pbjson,
response::constant_response,
Expand Down Expand Up @@ -567,7 +567,7 @@ pub struct Blobs {

#[must_use]
pub fn make_blobs(heights: &[u32]) -> Blobs {
use astria_core::generated::sequencerblock::v1::{
use astria_core::generated::astria::sequencerblock::v1::{
SubmittedMetadataList,
SubmittedRollupDataList,
};
Expand Down
2 changes: 1 addition & 1 deletion crates/astria-conductor/tests/blackbox/soft_only.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use astria_conductor::{
Conductor,
Config,
};
use astria_core::generated::execution::v1::{
use astria_core::generated::astria::execution::v1::{
GetCommitmentStateRequest,
GetGenesisInfoRequest,
};
Expand Down
6 changes: 6 additions & 0 deletions crates/astria-core/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Moved definitions of address domain type to `astria-core-address` and
reexported items using the same aliases [#1802](https://github.com/astriaorg/astria/pull/1802)

### Changed

- Move all Astria APIs generated from the Protobuf spec from `astria_core::generated`
to `astria_core::generated::astria`
[#1825](https://github.com/astriaorg/astria/pull/1825)

### Removed

- Removed method `TracePrefixed::last_channel` [#1768](https://github.com/astriaorg/astria/pull/1768)
Expand Down
8 changes: 4 additions & 4 deletions crates/astria-core/src/execution/v1/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use bytes::Bytes;
use pbjson_types::Timestamp;

use crate::{
generated::execution::v1 as raw,
generated::astria::execution::v1 as raw,
primitive::v1::{
IncorrectRollupIdLength,
RollupId,
Expand Down Expand Up @@ -43,7 +43,7 @@ enum GenesisInfoErrorKind {
#[cfg_attr(feature = "serde", derive(serde::Serialize))]
#[cfg_attr(
feature = "serde",
serde(into = "crate::generated::execution::v1::GenesisInfo")
serde(into = "crate::generated::astria::execution::v1::GenesisInfo")
)]
pub struct GenesisInfo {
/// The rollup id which is used to identify the rollup txs.
Expand Down Expand Up @@ -148,7 +148,7 @@ enum BlockErrorKind {
#[cfg_attr(feature = "serde", derive(serde::Serialize))]
#[cfg_attr(
feature = "serde",
serde(into = "crate::generated::execution::v1::Block")
serde(into = "crate::generated::astria::execution::v1::Block")
)]
pub struct Block {
/// The block number
Expand Down Expand Up @@ -380,7 +380,7 @@ impl CommitmentStateBuilder<WithFirm, WithSoft, WithCelestiaBaseHeight> {
#[cfg_attr(feature = "serde", derive(serde::Serialize))]
#[cfg_attr(
feature = "serde",
serde(into = "crate::generated::execution::v1::CommitmentState")
serde(into = "crate::generated::astria::execution::v1::CommitmentState")
)]
pub struct CommitmentState {
/// Soft commitment is the rollup block matching latest sequencer block.
Expand Down
Loading

0 comments on commit fc10a63

Please sign in to comment.