-
Notifications
You must be signed in to change notification settings - Fork 88
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
chore(proto): move optimistic block protos to v1 #1707
Conversation
b1711e8
to
f27c8d6
Compare
f27c8d6
to
00b2ae6
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
auctioneer isn't shipped yet, maybe this should stay v1alpha1 for now?
The auctioneer stuff remains v1alpha1 here, it is just updated to use the correct (v1) version of everything else. |
00b2ae6
to
acc9564
Compare
acc9564
to
9a65f20
Compare
Have to push back on this naming convention. v1 sequencer block with v1alpha1 optimistic block? We should have probably not nested the optimistic block bits under sequencerblock in this way, but provided a structure like in the protocol APIs. I am not sure how to best tackle this, but I feel like going with Would like to pull @joroshiba into this. |
9a65f20
to
b9e041c
Compare
b9e041c
to
76f6587
Compare
76f6587
to
711bba5
Compare
proto/sequencerblockapis/astria/sequencerblock/optimisticblock/v1alpha1/optimistic_block.proto
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, think this is good sane versioning for the proto. Agreed with Janis on naming service.proto
as a nit.
711bba5
to
d1edce9
Compare
d1edce9
to
4287b94
Compare
This unfortunately imports the wrong generated code and doesn't name it according to our convention. This PR should not have been merged. (It also lacks a CHANGELOG). |
…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
Summary
This updates the protos used for the auctioneer to be based on the new v1 protos.
Background
We bumped our proto definition versions from
v1alpha_
tov1
.Changes
sequencerblock/optimistic_block
file into its own subpackage in order to havesequencerblock.optimisticblock.v1alpha1
FilteredSequencerBlock
from v1Breaking Changelist
closes #1767