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): Default agoric wallet send to use --gas=auto #10447

Merged
merged 9 commits into from
Nov 12, 2024

Conversation

gibson042
Copy link
Member

@gibson042 gibson042 commented Nov 11, 2024

Description

Extracted from #10165.
When the agoric wallet send transaction inside psmSwap failed because the default gas limit was insufficient, the following waitUntilOfferResult never settled because the agoric follow -lF :published.wallet.${addr} inside its queryWallet was waiting forever. This PR fixes the root issue by updating agoric wallet send to use --gas=auto (while still preserving a3p-integration test coverage of the "default gas with automatic wallet provisioning" scenario), and also updates retryUntilCondition to Promise.race its operation against the interval timeout rather than just naïvely awaiting it and possibly getting hung up on permanent unsettlement, while still allowing the original promise to persist (specifically for such agoric follow applications).
It also incidentally cleans up tryISTBalances, which is used by checkSwapSucceeded after the vulnerable psmSwap calls.

  • fix(client-utils): Retry at least every other interval (even when a backing operation hangs, e.g. follow of an unprovisioned wallet)
  • chore(a3p-integration): Increase the verbosity of psmSwap
  • feat(agoric-cli): Block agoric wallet send on tx inclusion (-bblock)
  • test(a3p-integration): Update "swap into IST" to force the default gas limit
  • feat(agoric-cli): Add agoric wallet send gas limit options (defaulting to --gas=auto --gas-adjustment=1.2)

Security Considerations

None known.

Scaling Considerations

None known.

Documentation Considerations

The CLI is self-documenting:

$ agoric wallet send --help
Usage: agoric wallet send [options]

send a prepared offer

Options:
  --home <dir>                      agd application home directory
  --keyring-backend <os|file|test>  keyring's backend (os|file|test) (default "os")
  --from [address]                  address literal or name
  --offer [filename]                path to file with prepared offer
  --dry-run                         spit out the command instead of running it
  --gas                             gas limit; "auto" [default] to calculate automatically
  --gas-adjustment                  factor by which to multiply the limit calculated from
                                    --gas=auto [default 1]
  --verbose                         print command output
  -h, --help                        display help for command

Testing Considerations

The new --gas=auto default is used every except "swap into IST", which is now explicitly responsible for covering the "default gas with automatic wallet provisioning" scenario.

Upgrade Considerations

This will now detect any future change that pushes PSM swap gas consumption beyond the default limit.

@gibson042 gibson042 added the force:integration Force integration tests to run on PR label Nov 11, 2024
@gibson042 gibson042 requested a review from a team as a code owner November 11, 2024 19:28
Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

🙏

Copy link

cloudflare-workers-and-pages bot commented Nov 11, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 4016066
Status: ✅  Deploy successful!
Preview URL: https://9c9d0cd7.agoric-sdk.pages.dev
Branch Preview URL: https://gibson-2024-11-ci-improvemen.agoric-sdk.pages.dev

View logs

@gibson042 gibson042 changed the title fix(client-utils): Retry at least every other interval fix(agoric-cli): Default agoric wallet send to use --gas=auto Nov 12, 2024
@gibson042 gibson042 force-pushed the gibson-2024-11-ci-improvements branch from 3f76987 to 4016066 Compare November 12, 2024 20:45
@gibson042 gibson042 added the automerge:rebase Automatically rebase updates, then merge label Nov 12, 2024
@mergify mergify bot merged commit fb76d70 into master Nov 12, 2024
86 of 95 checks passed
@mergify mergify bot deleted the gibson-2024-11-ci-improvements branch November 12, 2024 21:22
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 force:integration Force integration tests to run on PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants