feature(charts): hermes chart fixes, bech32 updates, ibc bridge test by steezeburger · Pull Request #1130 · astriaorg/astria

steezeburger · 2024-06-01T00:26:25Z

Summary

Fixes hermes helm chart

Background

We want to run hermes via helm chart so we can automate e2e testing.

Changes

add 2nd funded celestia account
use this account's key in hermes
fix hermes chart values
add new chart bridge-test
add new justfile dev/bridgetester.just
add AstriaSequencerHrpPrefix to evm rollup geth-genesis

Testing

just deploy-cluster
just deploy-ibc-test-infra
- deploys ingress controller
- deploys celestia-local
- deploys single astria sequencer
  - ./charts/sequencer/values.yaml
  - ./dev/values/validators/single.yaml
- deploys evm rollup
  - ./charts/evm-rollup/values.yaml
  - ./dev/values/rollup/dev.yaml
  - ./dev/values/rollup/ibc-bridge-test.yaml
- deploy hermes
  - ./charts/hermes/values.yaml
  - ./dev/values/hermes/local.yml
just deploy-bridge-tester
- initContainer handles initializing bridge account on the sequencer
- container runs scripts
  - celestia-appd keys add
  - celestia-appd tx ibc-transfer transfer
  - checks balance before and after and fails if ibc-transfer did not go through

Metrics

List out metrics added by PR, delete section if none.

Breaking Changelist

Bulleted list of breaking changes, any notes on migration. Delete section if none.

Related Issues

Link any issues that are related, prefer full github links.

closes

* add celestia ibc account * fix some values

joroshiba · 2024-06-01T01:37:08Z

dev/values/hermes/local.yml

    gasPrice: 1
    gasDenom: nria
-    trustThreshold: 2/3
+    trustThreshold: 1/3


this should probably be 2/3 to match tendermint?

Updated!
I got these values from https://github.com/astriaorg/hermes/blob/astria/config-astria-celestia.toml
Should I update there as well? Not 100% sure of the implication/meaning of trustThreshold

The trust threshold defines what fraction of the total voting power of a known and trusted validator set is sufficient for a commit to be accepted going forward. - hermes docs

Basically 2/3 is the threshold for CometBFT to accept a block so I don't think it makes sense to accept lower than that.

dev/values/hermes/local.yml

* main: fix(charts): conductor configmap fix (#1146) feat(sequencer): add `allowed_fee_asset_ids` abci query and `sequencer_client` support (#1127) chore(conductor): release conductor 0.17 (#1139) feat: use macro to declare metric constants (#1129) refactor(merkle): remove source of panics in audit API (#1137) feat(conductor): skip outdated block metadata (#1120) refactor(sequencer): remove mint module (#1134) feat(bridge-withdrawer): add justfile (#1135) chore(chart): change evm back to latest on dev (#1132) feat(conductor, proto)!: celestia base heights in commitment state (#1121)

* main: fix: ignore RUSTSEC-2021-0139 (#1171) chore(sequencer-relayer)!: remove functionality to restrict relaying blocks to only those proposed by a given validator (#1168) chore(metrics): update `metric_name` macro to handle a collection of names (#1163) fix(bridge-withdrawer): skip linting generated contract code (#1172) fix(core, sequencer): prefix removal source non-refund ics20 packet (#1162) chore(docs): add sequencer-relayer doc to specs (#1126) feat(bridge-withdrawer): sync logic (#1165) chore(withdrawer): replace contracts with `astria-bridge-contracts` submodule (#1164) feat(sequencer)!: implement bridge sudo and withdrawer addresses (#1142) feat(sequencer): implement refund to rollup logic upon ics20 transfer refund (#1161) feat(bridge-withdrawer): bridge withdrawer startup (#1160) feat(core, proto)!: add bech32m addresses (#1124) feat(withdrawer): bridged ERC20 token withdrawals (#1149) feat(sequencer-relayer)!: add chain IDs for sequencer and Celestia to config env vars (#1063) test(bridge-withdrawer): add submitter tests (#1133) chore: bump penumbra deps (#1159) feat(sequencer): implement `bridge/account_last_tx_hash` abci query (#1158) fix(withdrawer): use block subscription in batcher; send to destination_chain_address (#1157) fix(withdrawer): update AstriaWithdrawer to check that withdrawal value is sufficient (#1148) chore(ci): build bridge withdrawer images (#1156)

testing astria-geth bech32 updates

* main: specs: add native bridging protocol doc (#1177) Chore(charts): update sequencer faucet chart (#1140) chore: remove unused dependencies (#1174) chore: bump msrv and run clippy (#1167)

* main: chore(bridge-withdrawer): add missing errors and clean up names (#1178) feat(sequencer): add ttl and invalid cache to app mempool (#1138) chore(astria-merkle): add benchmarks (#1179) chore(sequencer-relayer): add timeout to gRPCs to Celestia app (#1191) refactor(core): parse ics20 denoms as ibc or trace prefixed variants (#1181) Mycodecrafting/sequencer seed node (#1188) chore: register all metrics during startup (#1144) feat(charts): option to purge geth mempool (#1182)

* main: (27 commits) refactor(sequencer): fix prepare proposal metrics (#1211) refactor(bridge-withdrawer): move generated contract bindings to crate (#1237) fix(sequencer) fix wrong metric and remove unused metric (#1240) feat(sequencer): implement transaction fee query (#1196) chore(cli)!: remove unmaintained rollup subcommand (#1235) release(sequencer): 0.14.1 patch release (#1233) feat(sequencer-utils): generate example genesis state (#1224) feat(sequencer): implement abci query for bridge account info (#1189) feat(charts): bridge-withdrawer, smoke test, various chart improvements (#1141) chore(charts): update for new geth update (#1226) chore(chart)!: dusk-8 chart version updates (#1223) release(conductor): fix conductor release version (#1222) release: dusk-8 versions (#1219) fix(core): revert `From` ed25519_consensus types for crypto mod (#1220) Refactor(chart, sequencer): restructure sequencer chart, adjust configs (#1193) refactor(withdrawer): read from block subscription stream and get events on each block (#1207) feat(core): implement with verification key for address builder and crypto improvements (#1218) feat(proto, sequencer)!: use full IBC ICS20 denoms instead of IDs (#1209) chore(chart): update evm chart for new prefix field (#1214) chore: bump penumbra deps (#1216) ...

joroshiba · 2024-07-10T08:53:28Z

charts/celestia-local/templates/configmap.yaml

  bridge_rpc_port: "{{ .Values.ports.bridgeRPC }}"
  celestia_app_host_port: "{{ .Values.ports.celestiaAppHostPort }}"
  celestia_app_grpc_port: "{{ .Values.ports.celestiaAppGrpcPort }}"
+  ibc_account_mnemonic: "{{ .Values.ibcAccountMnemonic }}"


Should we maybe update to a generic structure here in values that is array of accounts with object fields name, mnemonic, value, staked_value? This would allow customizing the chart to generically generate genesis setup with any number of accounts.

joroshiba · 2024-07-10T08:58:39Z

charts/celestia-local/templates/ingress.yaml

+    nginx.ingress.kubernetes.io/enable-cors: "true"
+    # allow requests from bridge web app
+    nginx.ingress.kubernetes.io/cors-allow-origin: "http://localhost:3000"


Why do we need cors here?

charts/evm-rollup/files/genesis/geth-genesis.json

* main: feat(cli): add cmd to collect withdrawal events and submit as actions (#1261) fix(core, bridge, sequencer)!: dismabiguate return addresses (#1266) fix(withdrawer): support withdrawer address that differs from bridge address (#1262) (core, sequencer)!: generate serde traits impls for all protocol protobufs (#1260) fix(charts): add resources for sequencer/cometbft (#1254) chore(sequencer)!: add metrics (#1248) fix(sequencer-utils): fixes issue in `parse_blob` tests (#1243) feat(core, proto)!: make bridge unlock memo string (#1244) fix(conductor): don't panic during panic (#1252) feat(core)!: lowerCamelCase for protobuf json mapping (#1250) refactor(bridge-withdrawer)!: refactor startup to a separate subtask and remove balance check from startup (#1190) fix: rollup archive node configurations (#1249) refactor(core, bridge-withdrawer)!: move bridge-unlock memo to core (#1245) fix(sequencer)!: store native asset ibc->trace mapping in init_chain (#1242)

* main: chore(test): use a temporary file to not pollute the workspace (#1269) chore(sequencer): add mempool benchmarks (#1238) fix(bridge-withdrawer)!: fix nonce handling (#1215) feat(cli, bridge-withdrawer)!: share code between cli and service (#1270)

joroshiba

looks good enough to me, we can merge and then clean up

* main: chore(cli): remove unused rollup cli code (#1275)

* main: (24 commits) chore: update `bytes` and `ics23` crates (#1279) fix(sequencer): improve and fix instrumentation (#1255) feature(charts): hermes chart fixes, bech32 updates, ibc bridge test (#1130) chore(cli): remove unused rollup cli code (#1275) chore(test): use a temporary file to not pollute the workspace (#1269) chore(sequencer): add mempool benchmarks (#1238) fix(bridge-withdrawer)!: fix nonce handling (#1215) feat(cli, bridge-withdrawer)!: share code between cli and service (#1270) feat(cli): add cmd to collect withdrawal events and submit as actions (#1261) fix(core, bridge, sequencer)!: dismabiguate return addresses (#1266) fix(withdrawer): support withdrawer address that differs from bridge address (#1262) (core, sequencer)!: generate serde traits impls for all protocol protobufs (#1260) fix(charts): add resources for sequencer/cometbft (#1254) chore(sequencer)!: add metrics (#1248) fix(sequencer-utils): fixes issue in `parse_blob` tests (#1243) feat(core, proto)!: make bridge unlock memo string (#1244) fix(conductor): don't panic during panic (#1252) feat(core)!: lowerCamelCase for protobuf json mapping (#1250) refactor(bridge-withdrawer)!: refactor startup to a separate subtask and remove balance check from startup (#1190) fix: rollup archive node configurations (#1249) ...

…1130) ## Summary Fixes hermes helm chart ## Background We want to run hermes via helm chart so we can automate e2e testing. ## Changes * add 2nd funded celestia account * use this account's key in hermes * fix hermes chart values * add new chart `bridge-test` * add new justfile `dev/bridgetester.just` * add `AstriaSequencerHrpPrefix` to evm rollup geth-genesis ## Testing - `just deploy-cluster` - `just deploy-ibc-test-infra` - deploys ingress controller - deploys celestia-local - deploys single astria sequencer - `./charts/sequencer/values.yaml` - `./dev/values/validators/single.yaml` - deploys evm rollup - `./charts/evm-rollup/values.yaml` - `./dev/values/rollup/dev.yaml` - `./dev/values/rollup/ibc-bridge-test.yaml` - deploy hermes - `./charts/hermes/values.yaml` - `./dev/values/hermes/local.yml` - `just deploy-bridge-tester` - initContainer handles initializing bridge account on the sequencer - container runs scripts - `celestia-appd keys add` - `celestia-appd tx ibc-transfer transfer` - checks balance before and after and fails if ibc-transfer did not go through ## Metrics - List out metrics added by PR, delete section if none. ## Breaking Changelist - Bulleted list of breaking changes, any notes on migration. Delete section if none. ## Related Issues Link any issues that are related, prefer full github links. closes

steezeburger added 3 commits May 31, 2024 18:23

fix hermes chart

a488dc5

* add celestia ibc account * fix some values

reverse unneeded changes

b3519fd

reverse uneeded

4c7c2b5

steezeburger requested a review from a team as a code owner June 1, 2024 00:26

steezeburger requested a review from joroshiba June 1, 2024 00:26

github-actions bot added the cd label Jun 1, 2024

joroshiba reviewed Jun 1, 2024

View reviewed changes

steezeburger added 7 commits June 3, 2024 11:32

pr feedback

45e1282

add celestia app rest api to ingress, unsafe cors option

71a9b58

add new celestia dev account to genesis to be used with keplr

72de1cb

use new hermes image from registry

93f3d6f

increment celestia-local chart version

d408372

update hermes chart version as well

16dbb7e

steezeburger changed the title ~~Fix/hermes chart~~ fix(charts): hermes chart fixes Jun 5, 2024

steezeburger added 3 commits June 5, 2024 13:14

make comment more clear

0f261e3

bridge-test chart

fc1cf45

wip ibc testing

b2b4c1f

steezeburger requested a review from a team as a code owner June 10, 2024 17:27

steezeburger requested a review from sambukowski June 10, 2024 17:27

steezeburger added 5 commits June 10, 2024 11:27

consistent naming

49f6516

config updates, use script for job, configmap

143e83f

bech32

d985e57

human-readable prefix updates to geth

c8709df

testing astria-geth bech32 updates

steezeburger changed the title ~~fix(charts): hermes chart fixes~~ fix(charts): hermes chart fixes, bech32 updates, ibc bridge test Jun 14, 2024

steezeburger changed the title ~~fix(charts): hermes chart fixes, bech32 updates, ibc bridge test~~ feature(charts): hermes chart fixes, bech32 updates, ibc bridge test Jun 14, 2024

steezeburger added 3 commits June 14, 2024 15:02

Merge branch 'main' into fix/hermes-chart

a0b75b0

* main: specs: add native bridging protocol doc (#1177) Chore(charts): update sequencer faucet chart (#1140) chore: remove unused dependencies (#1174) chore: bump msrv and run clippy (#1167)

ibc bridge test

6044a64

changes for ci/cd

fcf2a9c

steezeburger added 11 commits June 18, 2024 22:44

shorter timeout

c1dc1bd

last commit didnt trigger action?

49882bd

revert

88b721a

trying to trigger action

bab039a

hm

fac1df6

set TAG envar

3569078

simplify gh job

36869ff

check status

79d44da

else not elif

0acf5d1

joroshiba reviewed Jul 10, 2024

View reviewed changes

steezeburger added 9 commits July 10, 2024 13:47

fix hermes smoke tests

4e7bca5

updates to hermes chart and ibc test

e7907df

remove extra spaces

01a3e86

remove extra line

63ef8b8

escape evm address correctly for memo field

e7c878a

fix chart maintainers list

7d0d246

version bump

54c9a8b

joroshiba approved these changes Jul 17, 2024

View reviewed changes

steezeburger added 2 commits July 17, 2024 09:52

Merge branch 'main' into fix/hermes-chart

79af7bf

* main: chore(cli): remove unused rollup cli code (#1275)

better default for just command

11a7ab7

steezeburger added this pull request to the merge queue Jul 17, 2024

Merged via the queue into main with commit bd34b38 Jul 17, 2024

steezeburger deleted the fix/hermes-chart branch July 17, 2024 08:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature(charts): hermes chart fixes, bech32 updates, ibc bridge test#1130

feature(charts): hermes chart fixes, bech32 updates, ibc bridge test#1130
steezeburger merged 52 commits intomainfrom
fix/hermes-chart

steezeburger commented Jun 1, 2024 •

edited

Loading

Uh oh!

joroshiba Jun 1, 2024

Uh oh!

steezeburger Jun 3, 2024

Uh oh!

joroshiba Jun 10, 2024

Uh oh!

Uh oh!

Uh oh!

joroshiba Jul 10, 2024

Uh oh!

joroshiba Jul 10, 2024

Uh oh!

Uh oh!

joroshiba left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

steezeburger commented Jun 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Background

Changes

Testing

Metrics

Breaking Changelist

Related Issues

Uh oh!

joroshiba Jun 1, 2024

Choose a reason for hiding this comment

Uh oh!

steezeburger Jun 3, 2024

Choose a reason for hiding this comment

Uh oh!

joroshiba Jun 10, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

joroshiba Jul 10, 2024

Choose a reason for hiding this comment

Uh oh!

joroshiba Jul 10, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joroshiba left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

steezeburger commented Jun 1, 2024 •

edited

Loading