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(agoric-cli): Thread rpcAddresses for Cosmos publishBundle #5660

Merged
merged 4 commits into from
Sep 3, 2022

Conversation

kriskowal
Copy link
Member

closes: #5601

Description

This change fixes #5601 which manifested as a failure to connect to a remote chain when publishing a bundle. This addresses the specific problem that publishBundle did not thread the rpcAddresses from the connection spec down to agd. This solution will be replaced with a system based on casting in cosmjs, with retries and feedback about installation success and failure #5460, but this addresses the problem until then.

@kriskowal kriskowal requested a review from dckc June 23, 2022 20:25
@dckc
Copy link
Member

dckc commented Jun 23, 2022

@vejmelkam would you please try this out? @ktomascz? @anilhelvaci?

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

I haven't verified that it lets folks deploy to devnet, but it looks worthwhile in any case.

*/
const choose = array => {
assert(array.length > 0);
const index = Math.floor(array.length * Math.random());
Copy link
Member

Choose a reason for hiding this comment

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

grumble... ambient authority...

Copy link
Member Author

Choose a reason for hiding this comment

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

Fair, and I can change this such that random is a power needed by makeBundlePublisher.

@kriskowal
Copy link
Member Author

@vejmelkam would you please try this out? @ktomascz? @anilhelvaci?

Issue #5601 should already be mitigated by Agoric/dapp-fungible-faucet#48 on agoric-sdk/master. That is to say, you should not see connection errors on agoric-sdk/master even without this change, because agoric init will use dapp-fungible-faucet/main as a template, and the template does not use publishBundle at this time.

To manually validate that this PR addresses the underlying problem requires invoking agoric init --dapp-branch 5660-repro dapp-test to use a back-dated version of the template, one which still uses publishBundle. That version has remaining issues with deployment because publishBundle does not retry if it misses a link in the block chain, but at least should not fail with connection-refused.

When we’ve ironed out the remaining issues with publishBundle, we intend to revisit the dapp-template such that it uses publishBundle again. @michaelfig and I have decided to refactor the contract deploy script in terms of @agoric/deploy-script-helpers/install.js, and to integrate bundle publishing there.

@ktomascz
Copy link

ktomascz commented Jun 24, 2022

@dckc @kriskowal

Hi, I have tested with the --dapp-branch 5660-repro dapp-test and it seems to be looking up the correct node but the address is assembled incorrectly:

➜  dapp-test git:(master) agoric deploy contract/deploy.js api/deploy.js
Open CapTP connection to ws://127.0.0.1:8000/private/captp...o
agoric: deploy: running /home/vscode/dapp-test/contract/deploy.js
agoric: deploy: Deploy script will run with Node.js ESM
agoric: deploy: /home/vscode/agoric-sdk/golang/cosmos/build/agd tx swingset install-bundle --gas auto --gas-adjustment 1.2 --home /home/vscode/dapp-test/_agstate/agoric-servers/testnet-8000/ag-cosmos-helper-statedir --node http://https://devnet.rpc.agoric.net:443 --keyring-backend test --from ag-solo --chain-id agoricdev-13 --yes @/tmp/agoric-cli-bundle--17567-ti44y7BVuJpX/bundle.json

Error: post failed: Post "http://https//devnet.rpc.agoric.net:443": dial tcp: lookup https: no such host
Usage:
  agd tx swingset install-bundle <JSON>/@<FILE>/- [flags]

Note the POST endpoint address: "http://https//devnet.rpc.agoric.net:443"

When I am testing this SDK branch with the default (main) dapp I am getting the same error as described in #5341
I am not sure these two issues are even related.

@kriskowal
Copy link
Member Author

Hi, I have tested with the --dapp-branch 5660-repro dapp-test and it seems to be looking up the correct node but the address is assembled incorrectly:

Thank you for the reproduction. I’ll look into making the code that produces the address more sophisticated, to handle both the bare address and full URL cases.

@kriskowal kriskowal force-pushed the 5601-connection-refused branch 4 times, most recently from c5f64cf to 208925c Compare July 26, 2022 23:52
@kriskowal kriskowal force-pushed the 5601-connection-refused branch 9 times, most recently from 2df40ac to 3ea06d4 Compare August 31, 2022 00:53
@kriskowal kriskowal force-pushed the 5601-connection-refused branch 7 times, most recently from e752371 to 44049b3 Compare September 1, 2022 21:46
@kriskowal kriskowal added the automerge:rebase Automatically rebase updates, then merge label Sep 3, 2022
@turadg turadg force-pushed the 5601-connection-refused branch from 44049b3 to 908f723 Compare September 3, 2022 18:57
@mergify mergify bot merged commit 6a3fc3f into master Sep 3, 2022
@mergify mergify bot deleted the 5601-connection-refused branch September 3, 2022 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:rebase Automatically rebase updates, then merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

connection refused when deploying dapp-fungible-faucet to devnet
3 participants