chore: merge main into feature branch #1866
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## Summary Updated the bridge spec to add more details on some of the data structures and link to the up-to-date actions. ## Background With the preparation for mainnet, some of the action names and locations have changed. The spec needed updating to reflect that. ## Related Issues Link any issues that are related, prefer full github links. closes <!-- list any issues closed here -->
## Summary adds ibc sudo address change command ## Background Cli currently does not support the `IbcSudoChange` action ## Changes - added `IbcSudoChange` command to the cli as sudo command ## Testing tested manually against dusk-11 network ## Related Issues part of #1474
Summary Adds bridge-sudo-change command for changing bridge privileges addresses Background Part of adding more actions to the cli Changes - adds `bridge-sudo-change` command setting bridge account privileges addresses - adds `bridge-account get` command querying bridge account information Testing Locally against a cluster Related Issues Part of #1474 closes #1547
## Summary fixes error in local rollup deployment through the cluster ## Background On running `just deploy-dev-rollup` helm throws an ingress pathType error. This PR fixes the error other warnings. ## Changes - bump blockscout app version - changes sitemap.xml pathType to `ImplementationSpecific` ## Testing locally against the cluster with smoke tests.
## Summary adds a configurable value to set celestia timeout_commit to `1s` speeding bridge-tests ## Background setting up hermes channels in CI bridge-test is slow, speeding up celestia blocktime will help reduce setup time. ## Changes added a `fast` flag to `celestia-local`, when set to true will configure celestia with a block time of `1s`. Configured `ibc-test` in CI to use the new flag. ## Testing against a local cluster --------- Co-authored-by: Jordan Oroshiba <jordan@astria.org>
## Summary This pull request addresses an issue with the naming conventions used in the local Docker build process. ## Background The build tags were previously not adhering to the expected naming scheme, leading to confusion and potential conflicts during chart deployment with local images using just commands. ## Changes Updated the Docker build script to ensure tags are generated according to the standard chart naming convention. ## Testing locally running and deploying the chart both using - `just docker-build-and-load astria-sequencer` - `just docker-build astria-sequencer` && `just load-inage sequencer` ## Related Issues closes #1743
## Summary This replaces `sed '...' <<< text` with `echo text | sed '...'` in the main justfile. ## Background On Ubuntu, the default shell doesn't recognise the redirect operator `<<<`, hence the `just` commands `docker-build` and `docker-build-and-load` both fail on Ubuntu. ## Changes - Replaced usage of `<<<` by piping `echo` output. ## Testing Ran the commands on Ubuntu and macOS. ## Changelogs No updates required.
## Summary updates celestia-node and celestia-local charts to support the new versions. ## Background Celestia mocha-4 testnet went through an upgrade, nodes should upgrade by nov 7th ## Changes - celestia-app `v2.0.0 -> v2.3.1` - celestia-node `v0.16.0-rc -> v0.18.3-mocha` - adjusts celestia-node` config.toml` env variables - adds pruner service to configs ## Testing celestia-local tested locally running smoke-tests, celestia-node sync tested locally & against flame. ## Breaking Changelist - celestia environment variable changes
## Summary Instructs CometBFT to index all ABCI events emitted by Sequencer. ## Background This change instructs CometBFT to index all events emitted by Sequencer by setting their field `index: true`. This allows programatically determining stuck IBC transactions to automatically trigger a timeout. Previously, the height that a given IBC packet was sent at had to be found manually before it could be passed to the Hermes CLI (Astria's IBC relayer of choice). The indexing field is described in the CometBFT ABCI spec [1]. 1: https://github.com/cometbft/cometbft/blob/16b1ed87c4b10118df10eb7175412db066bd09d5/spec/abci/abci%2B%2B_basic_concepts.md?plain=1#L318-L319 ## Changes - Index all event attributes before returning in `app::execute_transaction` ## Testing - Added unit test to ensure all event attributes are indexed for the events we create (unable to unit test an IBC action). ## Changelogs Changelog updated
## 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_` to `v1`. ## Changes - Moved the `sequencerblock/optimistic_block` file into its own subpackage in order to have `sequencerblock.optimisticblock.v1alpha1` - Updated to use `FilteredSequencerBlock` from v1 ## Breaking Changelist - This shouldn't break anything as none of these protos are used by anything in main currently. closes #1767
## Summary Ignore two new RustSec warnings. ## Background We get two non-critical warnings when running `cargo audit`: [RUSTSEC-2024-0384](https://rustsec.org/advisories/RUSTSEC-2024-0384) and [RUSTSEC-2024-0388](https://rustsec.org/advisories/RUSTSEC-2024-0388). <details><summary>RUSTSEC-2024-0384</summary><p> ``` Crate: instant Version: 0.1.13 Warning: unmaintained Title: `instant` is unmaintained Date: 2024-09-01 ID: RUSTSEC-2024-0384 URL: https://rustsec.org/advisories/RUSTSEC-2024-0384 Dependency tree: instant 0.1.13 ├── fastrand 1.9.0 │ └── futures-lite 1.13.0 │ ├── isahc 1.7.2 │ │ └── astria-sequencer-relayer 1.0.0 │ └── http-types 2.12.0 │ └── wiremock 0.5.22 │ ├── astria-sequencer-relayer 1.0.0 │ ├── astria-sequencer-client 0.1.0 │ │ ├── astria-sequencer-relayer 1.0.0 │ │ ├── astria-conductor 1.0.0 │ │ ├── astria-composer 1.0.0 │ │ ├── astria-cli 0.5.1 │ │ └── astria-bridge-withdrawer 1.0.1 │ ├── astria-conductor 1.0.0 │ ├── astria-composer 1.0.0 │ └── astria-bridge-withdrawer 1.0.1 ├── ethers-providers 2.0.14 │ ├── ethers-middleware 2.0.14 │ │ └── ethers 2.0.14 │ │ ├── astria-test-utils 0.1.0 │ │ │ └── astria-composer 1.0.0 │ │ ├── astria-composer 1.0.0 │ │ ├── astria-cli 0.5.1 │ │ ├── astria-bridge-withdrawer 1.0.1 │ │ └── astria-bridge-contracts 0.1.0 │ │ ├── astria-cli 0.5.1 │ │ └── astria-bridge-withdrawer 1.0.1 │ ├── ethers-contract 2.0.14 │ │ ├── ethers-middleware 2.0.14 │ │ └── ethers 2.0.14 │ └── ethers 2.0.14 └── ethers-middleware 2.0.14 ``` </p></details> <details><summary>RUSTSEC-2024-0388</summary><p> ``` Crate: derivative Version: 2.2.0 Warning: unmaintained Title: `derivative` is unmaintained; consider using an alternative Date: 2024-06-26 ID: RUSTSEC-2024-0388 URL: https://rustsec.org/advisories/RUSTSEC-2024-0388 Dependency tree: derivative 2.2.0 ├── penumbra-tct 0.80.7 │ ├── penumbra-txhash 0.80.7 │ │ └── penumbra-ibc 0.80.7 │ │ ├── astria-sequencer 1.0.0 │ │ └── astria-core 0.1.0 │ │ ├── astria-sequencer-utils 0.1.0 │ │ ├── astria-sequencer-relayer 1.0.0 │ │ ├── astria-sequencer-client 0.1.0 │ │ │ ├── astria-sequencer-relayer 1.0.0 │ │ │ ├── astria-conductor 1.0.0 │ │ │ ├── astria-composer 1.0.0 │ │ │ ├── astria-cli 0.5.1 │ │ │ └── astria-bridge-withdrawer 1.0.1 │ │ ├── astria-sequencer 1.0.0 │ │ ├── astria-core 0.1.0 │ │ ├── astria-conductor 1.0.0 │ │ ├── astria-composer 1.0.0 │ │ ├── astria-cli 0.5.1 │ │ ├── astria-bridge-withdrawer 1.0.1 │ │ └── astria-bridge-contracts 0.1.0 │ │ ├── astria-cli 0.5.1 │ │ └── astria-bridge-withdrawer 1.0.1 │ ├── penumbra-sct 0.80.7 │ │ └── penumbra-ibc 0.80.7 │ └── penumbra-keys 0.80.7 │ └── penumbra-sct 0.80.7 ├── penumbra-num 0.80.7 │ ├── penumbra-ibc 0.80.7 │ └── penumbra-asset 0.80.7 │ ├── penumbra-keys 0.80.7 │ └── penumbra-ibc 0.80.7 ├── penumbra-keys 0.80.7 ├── penumbra-asset 0.80.7 ├── ark-r1cs-std 0.4.0 │ ├── poseidon377 1.2.0 │ │ ├── penumbra-tct 0.80.7 │ │ ├── penumbra-sct 0.80.7 │ │ ├── penumbra-keys 0.80.7 │ │ └── penumbra-asset 0.80.7 │ ├── poseidon-permutation 1.1.0 │ │ └── poseidon377 1.2.0 │ ├── penumbra-tct 0.80.7 │ ├── penumbra-sct 0.80.7 │ ├── penumbra-num 0.80.7 │ ├── penumbra-keys 0.80.7 │ ├── penumbra-asset 0.80.7 │ └── decaf377 0.10.1 │ ├── poseidon377 1.2.0 │ ├── poseidon-permutation 1.1.0 │ ├── poseidon-parameters 1.1.0 │ │ ├── poseidon377 1.2.0 │ │ └── poseidon-permutation 1.1.0 │ ├── penumbra-tct 0.80.7 │ ├── penumbra-sct 0.80.7 │ ├── penumbra-num 0.80.7 │ ├── penumbra-keys 0.80.7 │ ├── penumbra-asset 0.80.7 │ ├── decaf377-rdsa 0.11.0 │ │ ├── penumbra-sct 0.80.7 │ │ ├── penumbra-proto 0.80.7 │ │ │ ├── penumbra-txhash 0.80.7 │ │ │ ├── penumbra-tct 0.80.7 │ │ │ ├── penumbra-sct 0.80.7 │ │ │ ├── penumbra-num 0.80.7 │ │ │ ├── penumbra-keys 0.80.7 │ │ │ ├── penumbra-ibc 0.80.7 │ │ │ ├── penumbra-asset 0.80.7 │ │ │ ├── astria-sequencer 1.0.0 │ │ │ └── astria-core 0.1.0 │ │ ├── penumbra-num 0.80.7 │ │ ├── penumbra-keys 0.80.7 │ │ └── penumbra-asset 0.80.7 │ ├── decaf377-ka 0.80.7 │ │ └── penumbra-keys 0.80.7 │ └── decaf377-fmd 0.80.7 │ ├── penumbra-proto 0.80.7 │ ├── penumbra-num 0.80.7 │ ├── penumbra-keys 0.80.7 │ └── penumbra-asset 0.80.7 ├── ark-poly 0.4.2 │ ├── ark-groth16 0.4.0 │ │ ├── poseidon377 1.2.0 │ │ ├── penumbra-num 0.80.7 │ │ └── decaf377 0.10.1 │ └── ark-ec 0.4.2 │ ├── poseidon377 1.2.0 │ ├── decaf377 0.10.1 │ ├── ark-r1cs-std 0.4.0 │ ├── ark-groth16 0.4.0 │ ├── ark-ed-on-bls12-377 0.4.0 │ │ ├── penumbra-tct 0.80.7 │ │ └── decaf377 0.10.1 │ ├── ark-crypto-primitives 0.4.0 │ │ └── ark-groth16 0.4.0 │ └── ark-bls12-377 0.4.0 │ ├── decaf377 0.10.1 │ └── ark-ed-on-bls12-377 0.4.0 ├── ark-ff 0.4.2 │ ├── ruint 1.12.3 │ │ └── celestia-types 0.1.1 │ │ ├── celestia-rpc 0.1.1 │ │ │ └── astria-conductor 1.0.0 │ │ ├── astria-sequencer-relayer 1.0.0 │ │ ├── astria-core 0.1.0 │ │ └── astria-conductor 1.0.0 │ ├── poseidon377 1.2.0 │ ├── poseidon-permutation 1.1.0 │ ├── penumbra-tct 0.80.7 │ ├── penumbra-sct 0.80.7 │ ├── penumbra-num 0.80.7 │ ├── penumbra-keys 0.80.7 │ ├── penumbra-ibc 0.80.7 │ ├── penumbra-asset 0.80.7 │ ├── decaf377-rdsa 0.11.0 │ ├── decaf377-ka 0.80.7 │ ├── decaf377-fmd 0.80.7 │ ├── decaf377 0.10.1 │ ├── ark-snark 0.4.0 │ │ ├── poseidon377 1.2.0 │ │ ├── penumbra-num 0.80.7 │ │ ├── decaf377 0.10.1 │ │ └── ark-crypto-primitives 0.4.0 │ ├── ark-relations 0.4.0 │ │ ├── poseidon377 1.2.0 │ │ ├── poseidon-permutation 1.1.0 │ │ ├── penumbra-tct 0.80.7 │ │ ├── penumbra-sct 0.80.7 │ │ ├── penumbra-num 0.80.7 │ │ ├── penumbra-keys 0.80.7 │ │ ├── penumbra-asset 0.80.7 │ │ ├── decaf377 0.10.1 │ │ ├── ark-snark 0.4.0 │ │ ├── ark-r1cs-std 0.4.0 │ │ ├── ark-groth16 0.4.0 │ │ └── ark-crypto-primitives 0.4.0 │ ├── ark-r1cs-std 0.4.0 │ ├── ark-poly 0.4.2 │ ├── ark-groth16 0.4.0 │ ├── ark-ed-on-bls12-377 0.4.0 │ ├── ark-ec 0.4.2 │ ├── ark-crypto-primitives 0.4.0 │ └── ark-bls12-377 0.4.0 ├── ark-ff 0.3.0 │ └── ruint 1.12.3 ├── ark-ec 0.4.2 └── ark-crypto-primitives 0.4.0 ``` </p></details> Given that the RustSec report doesn't suggest any concrete problems with either crate, and how difficult it will be to move away from these dependencies, I have just ignored these warnings in CI. ## Changes - Ignore RustSec warnings in `.cargo/audit.toml`. ## Testing Ran `cargo audit` locally. ## Changelogs No updates required - nothing really fixed or updated. ## Related Issues Closes #1796. Closes #1797.
…1689) ## Summary minor updates to the data verification and inclusion proof specs, mostly renaming/updating the types and some clarifications. ## Changes - update [specs/data-flow-and-verification.md](https://github.com/astriaorg/astria/compare/noot/specs?expand=1#diff-ec3cc86eef70a0285e3cd7fbc902eeb2ef082783cda5916b3d9b81f80e0b4917) - update [specs/sequencer-inclusion-proofs.md](https://github.com/astriaorg/astria/compare/noot/specs?expand=1#diff-0b52370ce97078002b7f4682377d6847fad36814fea97e7eab930e1edc9270ec)
## Summary removes immediate celestia balance check after performing ibc-transfer **from** celestia ## Background Current ibc-tests checks celestia wallet balance immediately after permorming an ibc transfer, this is redundant as we check the bablance change on the sequencer side. In addition to currently being broken duo to outdated expected value, the test currently passes randomly, most likely duo to latency. ## Changes - remove celestia balance checks from ibc transfer from celestia - moves ibc-transfer check in `run-without-native` above fee-asset change, avoiding latency issues. ## Testing - locally tested bad behavior by adding a 1s sleep prior to the first balance check ## Changelogs No updates required.
## Summary Adds `astria-cli sequencer fee-assets get` query command, getting all allowed fee-assets ## Background The astria CLI should have a query command for all abci query paths, the PR is part of an effort to accomplish this. It also enables more robust smoke-tests. ## Changes - Adds allowed fee assets query subcommand. ## Testing Manually runs the command against local, Dawn and mainnet. ## Changelogs Changelogs updated. ## Related Issues Link any issues that are related, prefer full GitHub links. closes #1815
## Summary Breaks out `astria_core::crypto` to crate `astria-core-crypto`. ## Background One needs to import the entire kitchensink that is `astria-core` even if one only wants to use a fraction of its types. That leads to extremely long compilation times because all of its dependencies need to be compiled. This patch is the start to breaking out parts of `astria-core` into their own free standing crates, which `astria-core` then reexports. A useful sideeffect of this change is that the coupling between the different parts of astria-core is reduced (for example, by removing the rarely used utility method `SigningKey::try_address`, allowing to decouple address and crypto logic). ## Changes - Move the module `astria_core::crypto` into new create `astria-core-crypto` and reexport under the `astria_core::crypto` alias (not breaking to consumers) - Remove the tight coupling between the crypto and address parts of the stack by removing the `SigningKey::try_address` (rarely used and only inside a few tests) method and moving `ADDRESS_LENGTH` to a thin `astria-core-consts` crate (this is a breaking change for users of `SigningKey::try_address`). ## Testing Tests for astria core's crypto refinement types were moved to `astria-core-crypto` and left unchanged. Sequencer tests were updated to use `Address::builder` instead of `SigningKey::try_address` and otherwise also left unchanged. ## Changelogs Changelogs updated. ## Breaking Changelist - These changes leave all services untouched. - These changes would be breaking the public API of `astria-core` due to the removal fo `SigningKey::try_address` but we don't provide any stability guarantees for it. ## Related Issues Part of #1798
## Summary
Breaks out address related logic in `astria_core::primitive::v1` to
`astria-core-address`
## Background
One needs to import the entire kitchensink that is `astria-core` even if
one only wants to use a fraction of its types. That leads to extremely
long compilation times because all of its dependencies need to be
compiled.
This patch is the start to breaking out parts of `astria-core` into
their own free standing crates, which `astria-core` then reexports.
## Changes
- Move address related types out of `astria_core::primitive::v1` into
`astria-core-address`
- Reexport all moved types under the same module (aliasing, where
necessary)
- Remove the serde `Serialize` and `Deserialize` impls on the domain
type `Address` (breaking because one needs to explicitly construct
protobuf/wire type `Address`; not breaking on the wire, json format
stays the same)
- Implement the `Protobuf` ("domain type") trait for `Address`, removing
its inherent constructors and methods to move from/to raw protobuf types
("wire types"); decouples `astria-core-address` from `astria-core`
(specifically, the generated protobuf types) (breaking for consumers
because they now need to import the `Protobuf` trait)
- Create a thin crate `astria-core-consts` that only contains the const
`ADDRESS_LENGTH` to allow decoupling of address and crypto logic
(addressed in a separate patch)
- Removed `AddressBuilder::with_iter` from the public interface (only
used inside the crate and not outside) (breaking for consumers of that
method)
## Testing
Tests were shuffled around but not removed. Snapshot tests stay in place
in astria core for now (because they are required for wire/protobuf
types).
## Changelogs
Changelogs updated.
## Breaking Changelist
- These changes leave all services untouched.
- If astria-core gave stability guarantees, this would be a breaking
change for consumers of `astria-core` because some inherent methods were
removed from its public API.
## Related Issues
Part of #1798
## Summary The hermes config didn't work when path filters were used because some standard config properties were below the seperated path filter section. ## Background Allow use of path filters on deployed hermes instances. ## Changelogs No updates required. --------- Co-authored-by: quasystaty <ido@astria.org>
## Summary Ensures deposits created within bridge lock actions use only trace-prefixed assets. ## Background Currently, deposits created within a bridge lock action include whichever asset is used in the action, which can be either IBC- or trace-prefixed. This poses a problem, since our EVM has no concept of IBC-prefixed assets, and we shouldn't expect others using our network to, either. One solution discussed was to enforce all `BridgeLock` actions to use trace prefixed assets, but this would be a consensus breaking change. This aims to remedy the bug whilst making no breaking changes. ## Changes - Map any IBC-prefixed assets included in a `BridgeLock` action to trace-prefixed when constructing the `Deposit`. ## Testing - Added unit test to ensure an IBC-prefixed asset included in a bridge lock action is correctly mapped to a trace-prefixed asset in the corresponding deposit. - Tested that change would be non-breaking by syncing a full node to mainnet ## Changelogs Changelog updated. ## Breaking Changes Technically, this change is breaking, since any sequencer version prior to this which received a `BridgeLock` action containing an IBC-prefixed asset would emit a `Deposit` with an IBC-prefixed asset, while any sequencer version after would only emit deposits with trace-prefixed assets. However, owing to the fact that no such cases have happened yet on mainnet, this change would not be practically breaking so long as no `BridgeLock` actions containing IBC-prefixed denoms have been submitted by the time this patch is pushed out. This is actively being tested by a full-node running this PR against mainnet. ## Steps to sync this PR to Mainnet on your local machine Ensure you have docker installed + running ### Download and build Sequencer ```bash git clone https://github.com/astriaorg/astria.git cd astria git checkout ENG-1004/trace_prefixed_deposits cargo build -p astria-sequencer --release just docker-build astria-sequencer ``` ### Helm install ```bash helm repo add astria https://astriaorg.github.io/charts/ ``` Download this file and put in the `astria` repo: https://github.com/astriaorg/networks/blob/main/astria/full-node-values.yaml Make these edits to `full-node-values.yaml`, replacing "your_moniker" with your chosen moniker: ```yaml namespace: "astria-node" moniker: `<your_moniker>` images: sequencer: tag: local cometbft: secrets: nodeKey: devContent: priv_key: value: 1yh4XrMHn75sSW5cOhGDTVgv5BbqXlhrLduxHcE2t1osbwKQzo7xlvSK1vh5CVDvHESPYK/56uTKXM/1ifqHbw== privValidatorKey: devContent: address: 8C17BBDC7C350C83C550163458FC9B7A5B54A64E pub_key: value: 4v1RdMiKkWgBBTTP26iRSLOEkAY99gMVfZijm6OCzjs= priv_key: value: WxsUPI2QASIwK+XV547R8xzL66a5oBR2G8h53t0Gcl7i/VF0yIqRaAEFNM/bqJFIs4SQBj32AxV9mKObo4LOOw== sequencer: metrics: enabled: false otel: enabled: false storage: local: true entities: sequencerSharedStorage: storageClassName: <your_moniker>-sequencer-shared-storage-local ``` From the `astria` repo: ```bash just deploy cluster just deploy ingress-controller just wait-for-ingress-controller just load-image sequencer helm install astria-full-node astria/sequencer --version 1.0.0-rc.4 \ --namespace astria-node --create-namespace -f full-node-values.yaml ``` After this you should be able to go into k9s and see both the sequencer and CometBFT running ## Related Issues closes #1806
…n_transaction` (#1746) ## Summary Changed all non-deposit related instances of `source_action_index` to `position_in_transaction` ## Background This is meant to be more descriptive and less confusing to consumers. Deposits should stay the same, though, for parity with Geth and to avoid breaking changes. ## Changes - Changed all instances of `source_action_index` to `position_in_transaction`, excluding deposits.. ## Testing Passing all tests ## Related Issues closes #1735
## Summary Moved all action and transaction handling to single module. ## Background Previously, all implementations of `ActionHandler` were done separately in components which corresponded to their given actions (or groups of actions, such as in `bridge`). While this may make sense from an action-centered point of view, it is confusing to an outsider to have to look many different places to find where stateless/stateful checks and execution occur. This PR consolidates all of the implementations as well as their testing. ## Changes - Created new `action_handler` module and moved the corresponding `ActionHandler` trait as well as all of its `impl`s into this module. - Renamed some moved tests for clarity. - Categorized tests by action in submodule `tests`. - Categorized impls by action in submodule `impls`. ## Testing Passing all tests. ## Changelogs Changelogs updated ## Related Issues closes #1657
## Summary Replace the 10-15-24 draft report with the final report which notes that we haven't resolved the 3 low severity fixes but have acknowledged them.
## Summary Changed `just docker-build astria-cli` command to match the CI's naming ## Background CI package name is "astria-cli", whereas the just command resulted in a docker image with only "cli". ## Changes - Changed command to result in `ghcr.io/astriaorg/astria-cli:local` ## Testing Manually tested ## Changelogs No updates needed ## Related Issues closes #1812
## Summary
Returns a all fee components at the latest height when sending an ABCI
info request containing the path `"fees/components"`.
## Background
Right now information on the sequencer state is very limited. This patch
allows getting a bit more info out of it.
## Changes
- Adds a handler for the path `"fees/components"` into the ABCI info
service running inside sequencer.
- The handler returns a JSON object containing the values for each
component. Each component can return one of the 3 states shown below:
```
{
"transaction": {
base: 1,
multiplier: 2,
},
"rollup_data_submission": "not set",
"ibc_relay": "<some error message",
}
```
## Testing
Provided an error for sending a high level ABCI info request to the
`Info` service.
…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 Removed an unnecessary return of a `Result` from the `fees` state extension trait. ## Background The method is infallible. ## Changes - Removed the return type from `add_fee_to_block_fees()` ## Testing Passing all tests. ## Changelogs No updates required ## Related Issues closes #1845
## Summary Removes the default RPC URL and chain IDs the CLI. ## Background It is a safety issue to interact with the wrong chain, and so these defaults should not be enshrined in the app. ## Changes - Remove the default value from all `--sequencer.chain-id` arguments. - Remove the default value from all `--sequencer-url` arguments. ## Testing All smoke tests already explicitly provided chain ID and RPC URL and thus keep working. Ran manually: omitting either arguments now results in errors like this: ``` error: the following required arguments were not provided: --sequencer-url <SEQUENCER_URL> ``` ## Changelogs "Changelogs updated." ## Breaking Changelist - All invocations of the CLI for interacting with Astria network `astria-dusk-10` at `"https://rpc.sequencer.dusk-10.devnet.astria.org"` no longer work. Chain ID and RPC URL now need to be set explicitly by providing an environment variable or command line argumient. Invocations that already had the chain ID and RPC URL present continue working unchanged.
## Summary Added names to all snapshot tests which did not have them. ## Background If a certain test had multiple snapshot assertions, they would be named based on their ordering. Hence any reordering, removal, or addition of the snapshots would break the test, which should not be the case. This change aims to fix this problem as well as establish a best practice of naming snapshots moving forward. ## Changes - Added names to all snapshot tests and regenerated all snapshots not within Conductor (due to code freeze) ## Testing Passing all tests. ## Related Issues closes #1656
## Summary This adds a snapshot test for the memo field being populated from the withdrawal info. ## Background The new test should serve to emphasize that the schema of the memo field is part of the API and changes to this could well be breaking. ## Changes - Moved some code unchanged inside the existing `memo_to_json` and removed the generic parameter. - Added snapshot test. ## Testing This _is_ a test. ## Changelogs No updates required - this is only adding a test. ## Related Issues Closes #1752. --------- Co-authored-by: noot <36753753+noot@users.noreply.github.com>
## Summary Added tests for the ABCI error codes. ## Background The new snapshot test should serve to emphasize that the error codes are part of the API and changes here would be breaking. The other test should help avoid accidental duplication of a value. ## Changes - Added a snapshot test. - Added a test for unique values of error codes. ## Testing These _are_ tests. ## Changelogs No updates required - this is only adding unit tests. ## Related Issues Closes #1634. Co-authored-by: noot <36753753+noot@users.noreply.github.com>
## Summary This reduces a lot of boilerplate around fees. ## Background The boilerplate code is very prone to copy-paste errors. Changing the 14 different (but structurally identical) `FeeComponent` structs into a single one with a generic arg reduces a lot of the boilerplate code. This PR replaces #1739 (an initial, more restricted refactor) and #1794 (a PR on top of #1739). It comprises the net changes of these two PRs in [the first commit](a8ec424), followed by most of the changes suggested by @eoroshiba's review of #1794 in [the second commit](c92dc98). [The third commit](1316e07) is also in response to @eoroshiba's suggestion to remove the fee aliases, and is a single commit in case others prefer that change to be reverted. ## Changes - In core, replaced fee component structs with a new `FeeComponents<T>` where `T` will be some action type. - In sequencer, changed the `FeeHandler` trait fairly radically to support the new form of fees. ## Testing Existing tests are sufficient - there should be no changed functionality to the business logic. ## Changelogs No updates required. ## Related Issues Closes #1715.
## Summary This is a simple merge of `main`. ## Background We want to keep the feature branch as in sync with `main` as possible. ## Changes - Merged `main`. Conflicts in the following files, but all relatively trivial: - crates/astria-cli/src/sequencer/bridge_account.rs - crates/astria-cli/src/sequencer/bridge_sudo_change.rs - crates/astria-cli/src/sequencer/fee_assets.rs - crates/astria-cli/src/sequencer/sudo/ibc_sudo_change.rs - crates/astria-core/CHANGELOG.md - crates/astria-core/Cargo.toml - crates/astria-core/src/generated/mod.rs - crates/astria-core/src/lib.rs - crates/astria-core/src/primitive/v1/mod.rs - crates/astria-core/src/protocol/genesis/v1.rs - crates/astria-core/src/sequencerblock/v1/block.rs - crates/astria-sequencer-utils/src/genesis_example.rs - crates/astria-sequencer/CHANGELOG.md - crates/astria-sequencer/src/app/mod.rs - crates/astria-sequencer/src/bridge/bridge_lock_action.rs - crates/astria-sequencer/src/fees/state_ext.rs - crates/astria-sequencer/src/grpc/storage/keys.rs - crates/astria-sequencer/src/sequencer.rs - justfile - tools/protobuf-compiler/src/main.rs ## Testing No new tests added. ## Changelogs No updates required. --------- Co-authored-by: Itamar Reif <9663129+itamarreif@users.noreply.github.com> Co-authored-by: quasystaty <ido@astria.org> Co-authored-by: Jordan Oroshiba <jordan@astria.org> Co-authored-by: Ethan Oroshiba <ethan@astria.org> Co-authored-by: noot <36753753+noot@users.noreply.github.com> Co-authored-by: Richard Janis Goldschmidt <github@aberrat.io> Co-authored-by: Josh Bowen <josh@astria.org>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
merge main into feat/oracle.
Changes
files with conflicts:
Testing
existing tests