Skip to content

Commit

Permalink
fix(agoric-cli): Default agoric wallet send to use --gas=auto (#1…
Browse files Browse the repository at this point in the history
…0447)

## Description
Extracted from #10165.
When the `agoric wallet send` transaction inside [`psmSwap`](https://github.com/Agoric/agoric-sdk/blob/e7af58fe25a802d23283633416d0900aa06a2127/a3p-integration/proposals/z%3Aacceptance/test-lib/psm-lib.js#L391) failed because the default gas limit was insufficient, the following [`waitUntilOfferResult`](https://github.com/Agoric/agoric-sdk/blob/e7af58fe25a802d23283633416d0900aa06a2127/packages/client-utils/src/sync-tools.js#L205) 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](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Promise/race) its operation against the interval timeout rather than just naïvely `await`ing 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:
```console
$ 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.
  • Loading branch information
mergify[bot] authored Nov 12, 2024
2 parents c236c55 + 4016066 commit fb76d70
Show file tree
Hide file tree
Showing 11 changed files with 1,222 additions and 83 deletions.
1 change: 1 addition & 0 deletions a3p-integration/proposals/z:acceptance/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"@endo/far": "^1.1.5",
"@endo/init": "^1.1.4",
"@endo/marshal": "^1.5.3",
"agoric": "dev",
"ava": "^6.1.2",
"execa": "^9.3.1",
"tsx": "^4.17.0"
Expand Down
5 changes: 3 additions & 2 deletions a3p-integration/proposals/z:acceptance/psm.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
initializeNewUser,
maxMintBelowLimit,
psmSwap,
sendOfferAgd,
} from './test-lib/psm-lib.js';
import { getBalances } from './test-lib/utils.js';

Expand Down Expand Up @@ -128,7 +129,7 @@ test.serial('initialize new user', async t => {
t.pass();
});

test.serial('swap into IST', async t => {
test.serial('swap into IST using agd with default gas', async t => {
const {
newUser: { name },
anchor,
Expand Down Expand Up @@ -161,7 +162,7 @@ test.serial('swap into IST', async t => {
'--feePct',
wantMintedFeeVal,
],
psmSwapIo,
{ ...psmSwapIo, sendOffer: sendOfferAgd },
);

await checkSwapSucceeded(t, metricsBefore, balancesBefore, {
Expand Down
158 changes: 129 additions & 29 deletions a3p-integration/proposals/z:acceptance/test-lib/psm-lib.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* eslint-env node */

import { execa } from 'execa';
import { getNetworkConfig } from 'agoric/src/helpers.js';
import {
boardSlottingMarshaller,
makeFromBoard,
Expand All @@ -26,6 +28,22 @@ import fsp from 'node:fs/promises';
import { NonNullish } from './errors.js';
import { getBalances } from './utils.js';

/** @import {Result as ExecaResult, ExecaError} from 'execa'; */
/**
* @typedef {ExecaResult & { all: string } & (
* | { failed: false }
* | Pick<
* ExecaError & { failed: true },
* 'failed',
* | 'shortMessage'
* | 'cause'
* | 'exitCode'
* | 'signal'
* | 'signalDescription'
* >
* )} SendOfferResult
*/

// Export these from synthetic-chain?
const USDC_DENOM = NonNullish(process.env.USDC_DENOM);
const PSM_PAIR = NonNullish(process.env.PSM_PAIR).replace('.', '-');
Expand Down Expand Up @@ -356,46 +374,126 @@ export const initializeNewUser = async (name, fund, io) => {
};

/**
* Similar to https://github.com/Agoric/agoric-3-proposals/blob/422b163fecfcf025d53431caebf6d476778b5db3/packages/synthetic-chain/src/lib/commonUpgradeHelpers.ts#L123-L139
* However, in the case where "address" is not provisioned "agoric wallet send" is needed because
* "agops perf satisfaction" tries to follow ":published.wallet.${address}" which blocks the execution because no such path exists in
* vstorage. In situations like this "agoric wallet send" seems a better choice as it doesn't depend on following user's vstorage wallet path
* Similar to
* https://github.com/Agoric/agoric-3-proposals/blob/422b163fecfcf025d53431caebf6d476778b5db3/packages/synthetic-chain/src/lib/commonUpgradeHelpers.ts#L123-L139
* However, for an address that is not provisioned, `agoric wallet send` is
* needed because `agops perf satisfaction` hangs when trying to follow
* nonexistent vstorage path ":published.wallet.${address}".
*
* @param {string} address
* @param {Promise<string>} offerPromise
* @returns {Promise<SendOfferResult>}
*/
export const sendOfferAgoric = async (address, offerPromise) => {
const offerPath = await mkTemp('agops.XXX');
const offer = await offerPromise;
await fsp.writeFile(offerPath, offer);

await agoric.wallet(
'--keyring-backend=test',
'send',
'--offer',
offerPath,
'--from',
address,
const [settlement] = await Promise.allSettled([
execa({
all: true,
})`agoric wallet --keyring-backend=test send --offer ${offerPath} --from ${address} --verbose`,
]);
return settlement.status === 'fulfilled'
? settlement.value
: settlement.reason;
};

/**
* A variant of {@link sendOfferAgoric} that uses `agd` directly to e.g.
* control gas calculation.
*
* @param {string} address
* @param {Promise<string>} offerPromise
* @returns {Promise<SendOfferResult>}
*/
export const sendOfferAgd = async (address, offerPromise) => {
const offer = await offerPromise;
const networkConfig = await getNetworkConfig({ env: process.env, fetch });
const { chainName, rpcAddrs } = networkConfig;
const args = [].concat(
[`--node=${rpcAddrs[0]}`, `--chain-id=${chainName}`],
[`--keyring-backend=test`, `--from=${address}`],
['tx', 'swingset', 'wallet-action', '--allow-spend', offer],
'--yes',
'-bblock',
'-ojson',
);

const [settlement] = await Promise.allSettled([
execa('agd', args, { all: true }),
]);

// Upon successful exit, verify that the *output* also indicates success.
// cf. https://github.com/Agoric/agoric-sdk/blob/master/packages/agoric-cli/src/lib/wallet.js
if (settlement.status === 'fulfilled') {
const result = settlement.value;
try {
const tx = JSON.parse(result.stdout);
if (tx.code !== 0) {
return { ...result, failed: true, shortMessage: `code ${tx.code}` };
}
} catch (err) {
return {
...result,
failed: true,
shortMessage: 'unexpected output',
cause: err,
};
}
}

return settlement.status === 'fulfilled'
? settlement.value
: settlement.reason;
};

/**
* @param {string} address
* @param {Array<any>} params
* @param {{
* follow: (...params: string[]) => Promise<object>;
* sendOffer?: (address: string, offerPromise: Promise<string>) => Promise<SendOfferResult>;
* setTimeout: typeof global.setTimeout;
* now: () => number
* }} io
*/
export const psmSwap = async (address, params, io) => {
const now = io.now();
const offerId = `${address}-psm-swap-${now}`;
const { now, sendOffer = sendOfferAgoric, ...waitIO } = io;
const offerId = `${address}-psm-swap-${now()}`;
const newParams = ['psm', ...params, '--offerId', offerId];
const offerPromise = executeCommand(agopsLocation, newParams);
await sendOfferAgoric(address, offerPromise);
const sendResult = await sendOffer(address, offerPromise);
if (sendResult.failed) {
const {
command,
durationMs,
shortMessage,
cause,
exitCode,
signal,
signalDescription,
all: output,
} = sendResult;
const summary = {
command,
durationMs,
shortMessage,
cause,
exitCode,
signal,
signalDescription,
output,
};
console.error('psmSwap tx send failed', summary);
throw Error(
`psmSwap tx send failed: ${JSON.stringify({ exitCode, signal, signalDescription })}`,
{ cause },
);
}
console.log('psmSwap tx send results', sendResult.all);

await waitUntilOfferResult(address, offerId, true, io, {
await waitUntilOfferResult(address, offerId, true, waitIO, {
errorMessage: `${offerId} not succeeded`,
});
};
Expand Down Expand Up @@ -427,27 +525,29 @@ const extractBalance = (balances, targetDenom) => {

/**
* Checking IST balances can be tricky because of the execution fee mentioned in
* https://github.com/Agoric/agoric-sdk/issues/6525. Here we first check with
* whatever is passed in. If the first attempt fails we try again to see if
* there was an execution fee charged. If still fails, we throw.
* https://github.com/Agoric/agoric-sdk/issues/6525. So we first check for
* equality, but if that fails we recheck against an assumption that a fee of
* the default "minFeeDebit" has been charged.
*
* @param {import('ava').ExecutionContext} t
* @param {number} actualBalance
* @param {number} expectedBalance
*/
export const tryISTBalances = async (t, actualBalance, expectedBalance) => {
const firstTry = await t.try(
(tt, actual, expected) => {
tt.deepEqual(actual, expected);
},
actualBalance,
expectedBalance,
);
const firstTry = await t.try(tt => {
tt.is(actualBalance, expectedBalance);
});
if (firstTry.passed) {
firstTry.commit();
return;
}

if (!firstTry.passed) {
firstTry.discard();
t.deepEqual(actualBalance + 200000, expectedBalance);
} else firstTry.commit();
firstTry.discard();
t.log('tryISTBalances assuming no batched IST fee', firstTry.errors);
// See golang/cosmos/x/swingset/types/default-params.go
// and `ChargeBeans` in golang/cosmos/x/swingset/keeper/keeper.go.
const minFeeDebit = 200_000;
t.is(actualBalance + minFeeDebit, expectedBalance);
};

/**
Expand Down
Loading

0 comments on commit fb76d70

Please sign in to comment.