Skip to content
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

Update CI E2E script to use hermes 0.15.0 keys add argument #2287

Closed

Conversation

PikachuEXE
Copy link
Contributor

@PikachuEXE PikachuEXE commented Jun 11, 2022

Closes: #XXX

Description


PR author checklist:

  • Added changelog entry, using unclog.
  • Added tests: integration (for Hermes) or unit/mock tests (for modules).
  • Linked to GitHub issue.
  • Updated code comments and documentation (e.g., docs/).

Reviewer checklist:

  • Reviewed Files changed in the GitHub PR explorer.
  • Manually tested (in case integration/unit/mock tests are absent).

From hermes 0.15.0

$ hermes keys add -h

hermes-keys-add
Adds a key to a configured chain

USAGE:
    hermes keys add [OPTIONS] --file <FILE> <CHAIN_ID>

ARGS:
    <CHAIN_ID>    identifier of the chain

OPTIONS:
    -f, --file <FILE>          path to the key file
    -h, --help                 Print help information
    -n, --name <NAME>          name of the key (defaults to the `key_name` defined in the config)
    -p, --hd-path <HD_PATH>    derivation path for this key [default: m/44'/118'/0'/0/0]

Problem is I have no idea if CI is supposed to be run with latest version or a range of versions
But if I run CI locally (with 0.15.0) then this change is required

I got this issue also from https://interchainacademy.cosmos.network/academy/ibc/hermesrelayer.html like #2286

Steps

Run E2E locally

  • Download https://github.com/informalsystems/ibc-rs/releases/tag/v0.15.0, version with suffix -x86_64-unknown-linux-gnu.tar.gz
  • Clone this repo, extract downloaded archive, copy bin hermes to cloned repo folder root
  • docker-compose -f ci/docker-compose-gaia-current.yml build relayer
  • docker-compose -f ci/docker-compose-gaia-current.yml up -d ibc-0 ibc-1 relayer
  • docker exec relayer /bin/sh -c /relayer/e2e.sh

Cleanup

  • docker-compose -f ci/docker-compose-gaia-current.yml down

@PikachuEXE
Copy link
Contributor Author

I am seeing hermes 0.15.0+4e83aae locally

But check for End to End testing (Gaia - v7.0.1) us using hermes 0.15.0+e463445

image

Copy link
Member

@adizere adizere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this was also fixed part of #2275 but it's a good idea to have it separately here also.

Thank you for the fix!

ci/e2e.sh Outdated
@@ -36,8 +36,8 @@ echo "Add keys for chains"
echo "-----------------------------------------------------------------------------------------------------------------"
hermes -c "$CONFIG_PATH" keys add "$CHAIN_A" -f user_seed_"$CHAIN_A".json
hermes -c "$CONFIG_PATH" keys add "$CHAIN_B" -f user_seed_"$CHAIN_B".json
hermes -c "$CONFIG_PATH" keys add "$CHAIN_A" -f user2_seed_"$CHAIN_A".json -k user2
hermes -c "$CONFIG_PATH" keys add "$CHAIN_B" -f user2_seed_"$CHAIN_B".json -k user2
hermes -c "$CONFIG_PATH" keys add "$CHAIN_A" -f user2_seed_"$CHAIN_A".json -n user2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it be --key-name ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After running

  • cargo build --release --bin hermes
  • ./target/release/hermes keys add -h
    It's indeed renamed (again?) to --key-name

Code updated

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's indeed renamed (again?) to --key-name

We only renamed it once recently, in this commit:

informalsystems/ibc-rs@a304c62

Now that I think about it, not sure what problem we're trying to solve in this PR. The ci/e2e.sh file already has the correct option, namely -k.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well if it works already on master then it's the tutorial's fault
I will close this PR now

…aster` branch not `0.15.0`)

```
$ ./target/release/hermes keys add -h
hermes-keys-add
Adds key to a configured chain or restores a key to a configured chain using a mnemonic

USAGE:
    hermes keys add [OPTIONS] --key-file <KEY_FILE> --mnemonic-file <MNEMONIC_FILE> <CHAIN_ID>

ARGS:
    <CHAIN_ID>    identifier of the chain

OPTIONS:
    -f, --key-file <KEY_FILE>
            path to the key file

    -h, --help
            Print help information

    -k, --key-name <KEY_NAME>
            name of the key (defaults to the `key_name` defined in the config)

    -m, --mnemonic-file <MNEMONIC_FILE>
            path to file containing mnemonic to restore the key from

    -p, --hd-path <HD_PATH>
            derivation path for this key [default: m/44'/118'/0'/0/0]
```
@PikachuEXE
Copy link
Contributor Author

CI works for master just not for 0.15.0 binary

@PikachuEXE PikachuEXE closed this Jun 15, 2022
@ljoss17
Copy link
Contributor

ljoss17 commented Jun 15, 2022

Hi @PikachuEXE, as the tutorial to run the CI locally tells you to take the v0.15.0 release, which is older than the commit a304c62, locally the docker for the e2e tests will use a Hermes version with the old flags -n or --name for the keys add command. But the CI will use a Hermes which uses the new flags -k or --key-name for the keys add command.
To run the e2e locally, instead of downloading the Hermes v0.15.0 and copying the bin hermes to the cloned repo folder root, you should build the hermes binary (cargo build --release --bin hermes) and copy that one (/target/debug/hermes) to the repo folder root.

@PikachuEXE
Copy link
Contributor Author

@ljoss17 Thanks for the tips

  1. I think you mean /target/release/hermes since cargo build --release --bin hermes is run
  2. I am using M1 Macbook so running cargo build --release --bin hermes will build arm64 version and give error like /relayer/e2e.sh: 19: /usr/bin/hermes: Exec format error
  3. Trying arch -x86_64 cargo build --release --bin hermes will give arch: posix_spawnp: cargo: Bad CPU type in executable, properly need to install x86 version to a separate path, run it with arch -x86_64
  4. Easiest way to run E2E right now probably just checkout the tag matching the downloaded version (But can't run master this way)

@adizere
Copy link
Member

adizere commented Jun 16, 2022

I am using M1 Macbook so running cargo build --release --bin hermes will build arm64 version and give error like /relayer/e2e.sh: 19: /usr/bin/hermes: Exec format error

Since you're running into these troubles, others will likely do as well.

We should document the correct way to do cargo build and debug+run Hermes on an M1 laptop. Is there a correct way?

I think @ljoss17 and @harveenSingh are both running M1 as well.

@PikachuEXE
Copy link
Contributor Author

Setup

# Rustup required
rustup target install x86_64-unknown-linux-gnu
# Homebrew required
brew tap messense/macos-cross-toolchains
brew install x86_64-unknown-linux-gnu

On Each Build

# Run these every time to find some way to set them permanently, e.g. .profile, direnv
export CC_x86_64_unknown_linux_gnu=x86_64-unknown-linux-gnu-gcc
export CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_LINKER=x86_64-unknown-linux-gnu-gcc

cargo build --release --bin hermes --target x86_64-unknown-linux-gnu
cp target/x86_64-unknown-linux-gnu/release/hermes ./

docker-compose -f ci/docker-compose-gaia-current.yml build relayer
docker-compose -f ci/docker-compose-gaia-current.yml up -d ibc-0 ibc-1 relayer
docker exec relayer /bin/sh -c /relayer/e2e.sh

Cleanup

docker-compose -f ci/docker-compose-gaia-current.yml down
rm ./hermes

References

@adizere
Copy link
Member

adizere commented Jun 24, 2022

This is very useful, thank you for sharing!

@harveenSingh is also on M1 and redoing some of these steps as part of https://github.com/informalsystems/ibc-rs/pull/2318 so copying him here to see if he encounters similar problems and reuse your recipe. We'll probably need to document these steps in our guide.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants