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

fix(pd): look up listenaddr from tm rpc #2602

Merged
merged 1 commit into from
May 19, 2023
Merged

Conversation

conorsch
Copy link
Contributor

@conorsch conorsch commented May 18, 2023

When joining a testnet, pd will inspect the RPC endpoint for the bootstrap node and copy its known peers into the generated Tendermint config. To date, since #1847, that logic has also interpolated an incorrect address for the bootstrap node (by default currently testnet.penumbra.zone:26657): the RPC address shouldn't be used, the P2P address should be. This is particularly important when as we move toward updating our configs to default to HTTPS: we can no longer assume that the host portion of a service URL will be the same between the RPC and P2P endpoints. Instead, let's use the RPC to inspect what the correct ListenAddress is.

Testing and review

First, verify the misbehavior on main branch:

git checkout main
cargo run --release --bin pd -- testnet unsafe-reset-all
cargo run --release --bin pd -- testnet join https://rpc.testnet.penumbra.zone
grep ^seed ~/.penumbra/testnet_data/node0/tendermint/config/config.toml

Observe that the first host in there is rpc.testnet.penumbra.zone:26656, which won't work: 26656 isn't even open on that IP. Next, try it on this feature branch:

git checkout smarter-seed-addr-lookup
cargo run --release --bin pd -- testnet unsafe-reset-all
cargo run --release --bin pd -- testnet join https://rpc.testnet.penumbra.zone
grep ^seed ~/.penumbra/testnet_data/node0/tendermint/config/config.toml

Observe that the first host in there is no longer a DNS name, but an IP address. Critically, the IP address is not the same as that for the RPC service:

❯ host rpc.testnet.penumbra.zone
rpc.testnet.penumbra.zone has address 34.111.241.130

Instead, the first node in the TM config should be 35.226.255.25:26656, which maps to the exposed p2p port for the tendermint service on fn-0 in the testnet deployment.

@conorsch conorsch temporarily deployed to smoke-test May 18, 2023 19:56 — with GitHub Actions Inactive
When joining a testnet, `pd` will inspect the RPC endpoint for the
bootstrap node and copy its known peers into the generated Tendermint
config. To date, since #1847, that logic has also interpolated an
incorrect address for the bootstrap node (by default currently
`testnet.penumbra.zone:26657`): the RPC address shouldn't be used, the
P2P address should be. This is particularly important when as we move
toward updating our configs to default to HTTPS: we can no longer assume
that the host portion of a service URL will be the same between the RPC
and P2P endpoints. Instead, let's use the RPC to inspect what the
correct ListenAddress is.
@conorsch conorsch force-pushed the smarter-seed-addr-lookup branch from bcc2ce9 to 40b85e2 Compare May 19, 2023 00:12
@conorsch conorsch temporarily deployed to smoke-test May 19, 2023 00:12 — with GitHub Actions Inactive
@conorsch conorsch marked this pull request as ready for review May 19, 2023 00:24
Copy link
Member

@zbuc zbuc left a comment

Choose a reason for hiding this comment

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

LGTM!

@conorsch conorsch merged commit 6d2b73d into main May 19, 2023
@conorsch conorsch deleted the smarter-seed-addr-lookup branch May 19, 2023 15:18
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.

2 participants