Skip to content

Commit

Permalink
feat(core, sequencer)!: make native asset optional (#1703)
Browse files Browse the repository at this point in the history
## Summary
Makes setting a native asset optional.

## Background
Native assets are a remnant of sequencer that is now deeply entrenched
in how it runs (whitebox) tests. While we don't want to set a native
assets, in mainnet, it is too big a lift to remove them at this point.

## Changes
- Change genesis validation to permit empty native assets.
- Update account chain init to only require native assets if accounts
are set.
- Remove native asset lookup from querying assets (this can be done via
looking up all ibc assets.

## Testing
The absence of native assets is not tested as of this patch. All tests
still function with native assets set.

A proper test would follow these steps:
1. Spin up a test net without native assets, accounts, or permitted fee
assets set
2. Bridge in an IBC asset via an ics20 transfer
3. Set a permitted fee asset to the bridged asset
4. Execute transactions.


## Breaking Changelist
This is marked as breaking since a node with before this change would
reject an empty native asset in its genesis while a node after this
patch would initialize successfully.

However, this is mild breakage because both nodes would not form a
network in the first place.

---------

Co-authored-by: quasystaty <[email protected]>
Co-authored-by: Jordan Oroshiba <[email protected]>
  • Loading branch information
3 people authored Oct 23, 2024
1 parent 43edb82 commit 3e16986
Show file tree
Hide file tree
Showing 10 changed files with 168 additions and 46 deletions.
34 changes: 33 additions & 1 deletion .github/workflows/docker-build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,38 @@ jobs:
TAG=sha-$(git rev-parse --short HEAD)
just ibc-test run $TAG
ibc-no-native-asset-test:
needs: [ run_checker, composer, conductor, sequencer, sequencer-relayer, evm-bridge-withdrawer, cli ]
if: (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'astriaorg/astria') && (github.event_name == 'merge_group' || needs.run_checker.outputs.run_docker == 'true')
runs-on: buildjet-8vcpu-ubuntu-2204
steps:
- uses: actions/checkout@v4
- name: Install just
uses: taiki-e/install-action@just
- name: Install kind
uses: helm/kind-action@v1
with:
install_only: true
- name: Log in to GHCR
uses: docker/login-action@v2
with:
registry: ghcr.io
username: ${{ github.actor }}
password: ${{ secrets.GITHUB_TOKEN }}
- name: Setup IBC Bridge Test Environment
timeout-minutes: 8
run: |
TAG=sha-$(git rev-parse --short HEAD)
just deploy cluster
kubectl create secret generic regcred --from-file=.dockerconfigjson=$HOME/.docker/config.json --type=kubernetes.io/dockerconfigjson
echo -e "\n\nDeploying with astria images tagged $TAG"
just ibc-test deploy-without-native $TAG
- name: Run IBC utia as native test
timeout-minutes: 3
run: |
TAG=sha-$(git rev-parse --short HEAD)
just ibc-test run-without-native $TAG
ibc-timeout-refund:
needs: [ run_checker, composer, conductor, sequencer, sequencer-relayer, evm-bridge-withdrawer, cli ]
if: (github.event_name != 'pull_request' || github.event.pull_request.head.repo.full_name == 'astriaorg/astria') && (github.event_name == 'merge_group' || needs.run_checker.outputs.run_docker == 'true')
Expand Down Expand Up @@ -264,7 +296,7 @@ jobs:
docker:
if: ${{ always() && !cancelled() }}
needs: [composer, conductor, sequencer, sequencer-relayer, evm-bridge-withdrawer, cli, smoke-test, smoke-cli, ibc-bridge-test, ibc-timeout-refund]
needs: [composer, conductor, sequencer, sequencer-relayer, evm-bridge-withdrawer, cli, smoke-test, smoke-cli, ibc-bridge-test, ibc-no-native-asset-test, ibc-timeout-refund]
uses: ./.github/workflows/reusable-success.yml
with:
success: ${{ !contains(needs.*.result, 'failure') }}
64 changes: 56 additions & 8 deletions charts/ibc-test.just
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,22 @@ delete:
just deploy hermes-local > /dev/null
kubectl wait -n astria-dev-cluster deployment hermes-local-chart --for=condition=Available=True --timeout=480s

@deploy-without-native tag=defaultTag:
echo "Deploying ingress controller..." && just deploy-ingress-controller > /dev/null
just wait-for-ingress-controller > /dev/null
echo "Deploying local celestia instance..." && just deploy celestia-local > /dev/null
helm dependency update ./sequencer > /dev/null
echo "Setting up single astria sequencer..." && helm install \
-n astria-validator-single single-sequencer-chart ./sequencer \
-f ../dev/values/validators/all-without-native.yml \
-f ../dev/values/validators/single.yml \
{{ if tag != '' { replace('--set images.sequencer.devTag=# --set sequencer-relayer.images.sequencerRelayer.devTag=#', '#', tag) } else { '' } }} \
--create-namespace > /dev/null
just wait-for-sequencer > /dev/null
echo "Deploying Hermes"
just deploy hermes-local > /dev/null
kubectl wait -n astria-dev-cluster deployment hermes-local-chart --for=condition=Available=True --timeout=480s

@deploy-timeout tag=defaultTag:
echo "Deploying ingress controller..." && just deploy-ingress-controller > /dev/null
just wait-for-ingress-controller > /dev/null
Expand All @@ -60,6 +76,36 @@ delete:
just deploy hermes-local > /dev/null
kubectl wait -n astria-dev-cluster deployment hermes-local-chart --for=condition=Available=True --timeout=480s

[no-cd]
run-without-native tag=defaultTag:
#!/usr/bin/env bash
set -e
ASTRIA_CLI_IMAGE="{{cli_image}}{{ if tag != '' { replace(':#', '#', tag) } else { '' } }}"

# Execute the transfer from Celestia to the Rollup
just ibc-test _do-ibc-transfer {{defaultNamespace}} {{sequencer_sudo_address}}

# Add transfer/channel-0/utia as fee-asset
docker run --rm --network host $ASTRIA_CLI_IMAGE sequencer sudo fee-asset add --private-key {{sequencer_sudo_pkey}} --asset transfer/channel-0/utia --sequencer-url {{sequencer_rpc_url}} --sequencer.chain-id {{sequencer_chain_id}}
# check that sequencer balance updated correctly

EXPECTED_BALANCE=$(echo "1 * {{transfer_amount}}" | bc)
for i in {1..50}
do
BALANCE=$(docker run --rm --network host $ASTRIA_CLI_IMAGE sequencer account balance {{sequencer_sudo_address}} --sequencer-url {{sequencer_rpc_url}} | awk '/transfer\/channel-0\/utia/{print $(NF-1)}')
echo "check $i, balance: $BALANCE, Expected: $EXPECTED_BALANCE"
if [ "$BALANCE" == "$EXPECTED_BALANCE" ]; then
expected_sequencer_balance_found="1"
break
else
sleep 1
fi
done
if [[ -z $expected_sequencer_balance_found ]]; then
echo "expected sequencer balance was not found after IBC transfer; IBC transfer with compat address failed"
exit 1
fi

[no-cd]
run tag=defaultTag:
Expand All @@ -73,7 +119,7 @@ run tag=defaultTag:
just init-ibc-bridge {{sequencer_tia_bridge_priv_key}} transfer/channel-0/utia nria {{tag}}

# Execute the transfer from Celestia to the Rollup
just ibc-test _do-ibc-transfer
just ibc-test _do-rollup-ibc-transfer

# Multiplication factor is 10^-6 (utia to tia) * 10^18 (rollup factor) = 10^12
let expected_balance="$initial_balance + {{transfer_amount}} * 10**12"
Expand All @@ -98,7 +144,7 @@ run tag=defaultTag:
fi

# Execute the transfer from Celstia to sequencer with compat address
just ibc-test _do-compat-ibc-transfer
just ibc-test _do-ibc-transfer {{defaultNamespace}} {{compat_address}}

# check that celestia balance updated correctly
for i in {1..50}
Expand Down Expand Up @@ -194,7 +240,6 @@ run tag=defaultTag:
sequencer_tia_bridge_address := "astria1d7zjjljc0dsmxa545xkpwxym86g8uvvwhtezcr"
eth_ws_url := "ws://ws-executor.astria.localdev.me/"
evm_contract_address := "0xA58639fB5458e65E4fA917FF951C390292C24A15"
sequencer_chain_id := "sequencer-test-chain-0"
[no-cd]
run-timeout tag=defaultTag:
#!/usr/bin/env bash
Expand All @@ -207,7 +252,7 @@ run-timeout tag=defaultTag:
just init-ibc-bridge {{sequencer_tia_bridge_priv_key}} transfer/channel-0/utia nria {{tag}}

# Execute the transfer from Celestia to the Rollup
just ibc-test _do-ibc-transfer
just ibc-test _do-rollup-ibc-transfer

# Multiplication factor is 10^-6 (utia to tia) * 10^18 (rollup factor) = 10^12
let expected_balance="$initial_balance + {{transfer_amount}} * 10**12"
Expand All @@ -233,7 +278,7 @@ run-timeout tag=defaultTag:
fi

# Execute the transfer from Celstia to sequencer with compat address
just ibc-test _do-compat-ibc-transfer
just ibc-test _do-ibc-transfer {{defaultNamespace}} {{compat_address}}

# check that celestia balance updated correctly
for i in {1..50}
Expand Down Expand Up @@ -371,8 +416,11 @@ run-timeout tag=defaultTag:

bridge_address := "astria1d7zjjljc0dsmxa545xkpwxym86g8uvvwhtezcr"
sequencer_address := "astria1cewd7alwml4fhx3w3lxq3vgf20cqe0qm650fac"
sequencer_sudo_pkey := "2bd806c97f0e00af1a1fc3328fa763a9269723c8db8fac4f93af71db186d6e90"
sequencer_sudo_address := "astria1rsxyjrcm255ds9euthjx6yc3vrjt9sxrm9cfgm"
compat_address := "astriacompat1cewd7alwml4fhx3w3lxq3vgf20cqe0qmdzxmvn"
celestia_dev_account_address := "celestia1m0ksdjl2p5nzhqy3p47fksv52at3ln885xvl96"
sequencer_chain_id := "sequencer-test-chain-0"
celestia_chain_id := "celestia-local-0"
celestia_node_url := "http://rpc.app.celestia.localdev.me:80"
sequencer_tia_bridge_priv_key := "6015fbe1c365d3c5ef92dc891db8c5bb26ad454bec2db4762b96e9f8b2430285"
Expand Down Expand Up @@ -403,7 +451,7 @@ transfer_fees := "26000"
# TODO: move this to deploy.just so that defaultNamespace need not be redefined
defaultNamespace := "astria-dev-cluster"
[no-cd]
_do-ibc-transfer namespace=defaultNamespace:
_do-rollup-ibc-transfer namespace=defaultNamespace:
echo "Performing IBC transfer..."
kubectl exec -n {{namespace}} pods/celestia-local-0 celestia-app -- /bin/bash -c \
'celestia-appd tx ibc-transfer transfer \
Expand All @@ -420,13 +468,13 @@ _do-ibc-transfer namespace=defaultNamespace:
--home /home/celestia \
--keyring-backend="{{keyring_backend}}"'

_do-compat-ibc-transfer namespace=defaultNamespace:
_do-ibc-transfer namespace=defaultNamespace toAddress="":
echo "Performing IBC transfer with compat address..."
kubectl exec -n {{namespace}} pods/celestia-local-0 celestia-app -- /bin/bash -c \
'celestia-appd tx ibc-transfer transfer \
transfer \
channel-0 \
{{compat_address}} \
{{toAddress}} \
"{{transfer_amount}}utia" \
--chain-id="{{celestia_chain_id}}" \
--from="{{celestia_dev_account_address}}" \
Expand Down
2 changes: 1 addition & 1 deletion charts/sequencer/Chart.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ type: application
# This is the chart version. This version number should be incremented each time you make changes
# to the chart and its templates, including the app version.
# Versions are expected to follow Semantic Versioning (https://semver.org/)
version: 1.0.0-rc.2
version: 1.0.0-rc.3
# This is the version number of the application being deployed. This version number should be
# incremented each time you make changes to the application. Versions are not expected to
# follow Semantic Versioning. They should reflect the version the application is using.
Expand Down
1 change: 0 additions & 1 deletion charts/sequencer/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ genesis:
base: "astria"
ibcCompat: "astriacompat"
authoritySudoAddress: ""
nativeAssetBaseDenomination: nria
allowedFeeAssets: []
# - nria
ibc:
Expand Down
23 changes: 16 additions & 7 deletions crates/astria-core/src/protocol/genesis/v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ pub struct GenesisAppState {
authority_sudo_address: crate::primitive::v1::Address,
ibc_sudo_address: crate::primitive::v1::Address,
ibc_relayer_addresses: Vec<crate::primitive::v1::Address>,
native_asset_base_denomination: asset::TracePrefixed,
native_asset_base_denomination: Option<asset::TracePrefixed>,
ibc_parameters: IBCParameters,
allowed_fee_assets: Vec<asset::Denom>,
fees: GenesisFees,
Expand Down Expand Up @@ -91,8 +91,8 @@ impl GenesisAppState {
}

#[must_use]
pub fn native_asset_base_denomination(&self) -> &asset::TracePrefixed {
&self.native_asset_base_denomination
pub fn native_asset_base_denomination(&self) -> Option<&asset::TracePrefixed> {
self.native_asset_base_denomination.as_ref()
}

#[must_use]
Expand Down Expand Up @@ -196,9 +196,16 @@ impl Protobuf for GenesisAppState {
.collect::<Result<_, _>>()
.map_err(Self::Error::ibc_relayer_addresses)?;

let native_asset_base_denomination = native_asset_base_denomination
.parse()
.map_err(Self::Error::native_asset_base_denomination)?;
let native_asset_base_denomination = if native_asset_base_denomination.is_empty() {
None
} else {
Some(
native_asset_base_denomination
.parse()
.map_err(Self::Error::native_asset_base_denomination),
)
.transpose()?
};

let ibc_parameters = {
let params = ibc_parameters
Expand Down Expand Up @@ -255,7 +262,9 @@ impl Protobuf for GenesisAppState {
chain_id: chain_id.clone(),
ibc_sudo_address: Some(ibc_sudo_address.to_raw()),
ibc_relayer_addresses: ibc_relayer_addresses.iter().map(Address::to_raw).collect(),
native_asset_base_denomination: native_asset_base_denomination.to_string(),
native_asset_base_denomination: native_asset_base_denomination
.as_ref()
.map_or(String::new(), ToString::to_string),
ibc_parameters: Some(ibc_parameters.to_raw()),
allowed_fee_assets: allowed_fee_assets.iter().map(ToString::to_string).collect(),
fees: Some(fees.to_raw()),
Expand Down
18 changes: 10 additions & 8 deletions crates/astria-sequencer/src/accounts/component.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,16 @@ impl Component for AccountsComponent {
where
S: accounts::StateWriteExt + assets::StateReadExt,
{
let native_asset = state
.get_native_asset()
.await
.wrap_err("failed to read native asset from state")?;
for account in app_state.accounts() {
state
.put_account_balance(&account.address, &native_asset, account.balance)
.wrap_err("failed writing account balance to state")?;
if !app_state.accounts().is_empty() {
let native_asset = state
.get_native_asset()
.await
.wrap_err("failed to read native asset from state")?;
for account in app_state.accounts() {
state
.put_account_balance(&account.address, &native_asset, account.balance)
.wrap_err("failed writing account balance to state")?;
}
}
Ok(())
}
Expand Down
17 changes: 4 additions & 13 deletions crates/astria-sequencer/src/accounts/query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,21 +55,12 @@ async fn get_trace_prefixed_account_balances<S: StateRead>(
let stream = state
.account_asset_balances(address)
.map_ok(|asset_balance| async move {
let native_asset = state
.get_native_asset()
let denom = ibc_to_trace(state, &asset_balance.asset)
.await
.context("failed to read native asset from state")?;

let result_denom = if asset_balance.asset == native_asset.to_ibc_prefixed() {
native_asset.into()
} else {
ibc_to_trace(state, &asset_balance.asset)
.await
.context("failed to map ibc prefixed asset to trace prefixed")?
.into()
};
.context("failed to map ibc prefixed asset to trace prefixed")?
.into();
Ok(AssetBalance {
denom: result_denom,
denom,
balance: asset_balance.balance,
})
})
Expand Down
15 changes: 8 additions & 7 deletions crates/astria-sequencer/src/app/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -288,13 +288,14 @@ impl App {
.put_ibc_compat_prefix(genesis_state.address_prefixes().ibc_compat().to_string())
.wrap_err("failed to write ibc-compat prefix to state")?;

let native_asset = genesis_state.native_asset_base_denomination();
state_tx
.put_native_asset(native_asset.clone())
.wrap_err("failed to write native asset to state")?;
state_tx
.put_ibc_asset(native_asset.clone())
.wrap_err("failed to commit native asset as ibc asset to state")?;
if let Some(native_asset) = genesis_state.native_asset_base_denomination() {
state_tx
.put_native_asset(native_asset.clone())
.wrap_err("failed to write native asset to state")?;
state_tx
.put_ibc_asset(native_asset.clone())
.wrap_err("failed to commit native asset as ibc asset to state")?;
}

state_tx
.put_chain_id_and_revision_number(chain_id.try_into().context("invalid chain ID")?)
Expand Down
1 change: 1 addition & 0 deletions crates/astria-sequencer/src/service/info/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@ mod tests {

state.put_base_prefix("astria".to_string()).unwrap();
state.put_native_asset(crate::test_utils::nria()).unwrap();
state.put_ibc_asset(crate::test_utils::nria()).unwrap();

let address = state
.try_base_prefixed(&hex::decode("a034c743bed8f26cb8ee7b8db2230fd8347ae131").unwrap())
Expand Down
39 changes: 39 additions & 0 deletions dev/values/validators/all-without-native.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
global:
dev: true

genesis:
chainId: 'sequencer-test-chain-0'
genesisTime: '2023-09-22T17:22:35.092832Z'
addressPrefixes:
base: "astria"
authoritySudoAddress: astria1rsxyjrcm255ds9euthjx6yc3vrjt9sxrm9cfgm
ibc:
enabled: true
inboundEnabled: true
outboundEnabled: true
sudoAddress: astria1rsxyjrcm255ds9euthjx6yc3vrjt9sxrm9cfgm
relayerAddresses:
- astria1rsxyjrcm255ds9euthjx6yc3vrjt9sxrm9cfgm
- astria1xnlvg0rle2u6auane79t4p27g8hxnj36ja960z
# Note large balances must be strings support templating with the u128 size
# account balances
genesisAccounts: []

resources:
cometbft:
requests:
cpu: 1000m
memory: 500Mi
limits:
cpu: 1000m
memory: 500Mi
sequencer:
requests:
cpu: 1000m
memory: 500Mi
limits:
cpu: 1000m
memory: 500Mi

storage:
enabled: false

0 comments on commit 3e16986

Please sign in to comment.