Skip to content

Commit

Permalink
chore(a3p): replace waitForBlock with retryUntilCondition on acceptan…
Browse files Browse the repository at this point in the history
…ce tests (#10404)

closes: https://github.com/Agoric/BytePitchPartnerEng/issues/10



## Description


This PR intends to replace the use of the `waitForBlock` method with the available `sync tools`, more specifically, the `retryUntilCondition` method.

The tests that were updated are:

- wallet.test.js
- core-eval.test.js
- value.Vow.test.js
- localchain.test.js

Note:

At wallet.test.js, the script `exitOffer.js` was replaced with the `broadcastBridgeAction` method of `walletUtils` to execute a `tryExitOffer` call.

### Security Considerations


No considerations.

### Scaling Considerations

No considerations.

### Documentation Considerations

No considerations.

### Testing Considerations


To build a condition that verifies the success of a `tryExitOffer` transaction for a failed or "bad invitation" offer, it was not possible to use the users wallet `liveOffers` due to the fact that when an invalid offer is created, the payment makes it to `liveOfferPayments` via `withdrawGive` but the offer fails to enter `liveOffers` or `liveOfferSeats`. 

Checking the Vstorage node `published.wallet.{addr}` for the status of `tryExitOffer` call also does not provide the needed information due to the error thrown by `tryExitOffer`, as the `offerId` isn’t present at `liveOfferSeats`.  

The alternative approach used was to monitor wallet balance, since the main goal is to ensure that the user’s payment is reclaimed,

Suggestion:  
If feasible, consider updating `Vstorage` to log a `tryExitOffer` attempt, even if it throws an error due to a missing `offerId`. This could include additional information about the transaction's outcome, such as “reclaim successful”.
This may require changes to `Vstorage` but could provide more transparent access to the outcome of similar scenarios.

This topic was addressed in the following discussion https://github.com/Agoric/agoric-sdk/discussions/10374

### Upgrade Considerations

No considerations.
  • Loading branch information
mergify[bot] authored Nov 6, 2024
2 parents d603b6c + ef55f0b commit 8943c95
Show file tree
Hide file tree
Showing 5 changed files with 64 additions and 140 deletions.
14 changes: 11 additions & 3 deletions a3p-integration/proposals/z:acceptance/core-eval.test.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
/* global setTimeout */

import test from 'ava';
import { readFile, writeFile } from 'node:fs/promises';

import { agd, evalBundles, waitForBlock } from '@agoric/synthetic-chain';
import { agd, evalBundles } from '@agoric/synthetic-chain';
import { retryUntilCondition } from './test-lib/sync-tools.js';

const SUBMISSION_DIR = 'core-eval-test-submission';

Expand Down Expand Up @@ -45,7 +48,12 @@ test(`core eval works`, async t => {

await evalBundles(SUBMISSION_DIR);

await waitForBlock(2); // enough time for core eval to execute ?
const actualValue = await retryUntilCondition(
async () => readPublished(nodePath),
value => value === nodeValue,
'core eval not processed yet',
{ setTimeout, retryIntervalMs: 5000, maxRetries: 15 },
);

t.is(await readPublished(nodePath), nodeValue);
t.is(actualValue, nodeValue);
});
98 changes: 0 additions & 98 deletions a3p-integration/proposals/z:acceptance/exitOffer.js

This file was deleted.

15 changes: 11 additions & 4 deletions a3p-integration/proposals/z:acceptance/localchain.test.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
import test from 'ava';
/* global setTimeout */

import { agd, evalBundles, waitForBlock } from '@agoric/synthetic-chain';
import test from 'ava';
import { agd, evalBundles } from '@agoric/synthetic-chain';
import { retryUntilCondition } from './test-lib/sync-tools.js';

const SUBMISSION_DIR = 'localchaintest-submission';

Expand All @@ -27,7 +29,12 @@ test(`localchain passes tests`, async t => {
const nodePath = 'test.localchain';
const nodeValue = JSON.stringify({ success: true });

await waitForBlock(2); // enough time for core eval to execute ?
const actualValue = await retryUntilCondition(
async () => readPublished(nodePath),
value => value === nodeValue,
'core eval not processed yet',
{ setTimeout, retryIntervalMs: 5000, maxRetries: 15 },
);

t.is(await readPublished(nodePath), nodeValue);
t.is(actualValue, nodeValue);
});
40 changes: 18 additions & 22 deletions a3p-integration/proposals/z:acceptance/valueVow.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,12 @@ import '@endo/init/debug.js';
import {
evalBundles,
getIncarnation,
waitForBlock,
GOV1ADDR as GETTER, // not particular to governance, just a handy wallet
GOV2ADDR as SETTER, // not particular to governance, just a handy wallet
} from '@agoric/synthetic-chain';
import { makeWalletUtils } from './test-lib/wallet.js';
import { networkConfig } from './test-lib/index.js';
import { retryUntilCondition } from './test-lib/sync-tools.js';

const START_VALUEVOW_DIR = 'start-valueVow';
const RESTART_VALUEVOW_DIR = 'restart-valueVow';
Expand Down Expand Up @@ -44,21 +44,21 @@ test('vow survives restart', async t => {
});

t.log('confirm the value is not in offer results');
await waitForBlock(2);
{
/** @type {any} */
const getterStatus = await walletUtils.readLatestHead(
`published.wallet.${GETTER}`,
);
console.log('current: ', inspect(getterStatus, { depth: 10 }));
t.like(getterStatus, {
status: {
id: 'get-value',
},
updated: 'offerStatus',
});
t.false('result' in getterStatus.status, 'no result yet');
}
let getterStatus = await retryUntilCondition(
async () => walletUtils.readLatestHead(`published.wallet.${GETTER}`),
value => value.status.id === 'get-value' && value.updated === 'offerStatus',
'Offer get-value not succeeded',
{ setTimeout, retryIntervalMs: 5000, maxRetries: 15 },
);

console.log('current: ', inspect(getterStatus, { depth: 10 }));
t.like(getterStatus, {
status: {
id: 'get-value',
},
updated: 'offerStatus',
});
t.false('result' in getterStatus.status, 'no result yet');

t.log('restart valueVow');
await evalBundles(RESTART_VALUEVOW_DIR);
Expand All @@ -82,11 +82,7 @@ test('vow survives restart', async t => {
});

t.log('confirm the value is now in offer results');
{
const getterStatus = await walletUtils.readLatestHead(
`published.wallet.${GETTER}`,
);
getterStatus = await walletUtils.readLatestHead(`published.wallet.${GETTER}`);

t.like(getterStatus, { status: { result: offerArgs.value } });
}
t.like(getterStatus, { status: { result: offerArgs.value } });
});
37 changes: 24 additions & 13 deletions a3p-integration/proposals/z:acceptance/wallet.test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/* global fetch setTimeout */

import test from 'ava';
import '@endo/init';
import {
Expand All @@ -6,14 +8,21 @@ import {
GOV1ADDR,
GOV2ADDR,
CHAINID,
waitForBlock,
} from '@agoric/synthetic-chain';
import { $ } from 'execa';
import { execFileSync } from 'child_process';
import {
agd,
getBalances,
replaceTemplateValuesInFile,
} from './test-lib/utils.js';
import { retryUntilCondition } from './test-lib/sync-tools.js';
import { makeWalletUtils } from './test-lib/wallet.js';
import { networkConfig } from './test-lib/index.js';

const walletUtils = await makeWalletUtils(
{ setTimeout, execFileSync, fetch },
networkConfig,
);

test.serial(`send invitation via namesByAddress`, async t => {
const SUBMISSION_DIR = 'invitation-test-submission';
Expand Down Expand Up @@ -43,20 +52,23 @@ test.serial(`send invitation via namesByAddress`, async t => {
});

test.serial('exitOffer tool reclaims stuck payment', async t => {
const offerId = 'bad-invitation-15'; // offer submitted on proposal upgrade-15 with an incorrect method name
const from = 'gov1';

const before = await getBalances([GOV1ADDR], 'uist');
t.log('uist balance before:', before);
const istBalanceBefore = await getBalances([GOV1ADDR], 'uist');

await $`node ./exitOffer.js --id ${offerId} --from ${from} `;
await waitForBlock(2);
const offerId = 'bad-invitation-15'; // offer submitted on proposal upgrade-15 with an incorrect method name
await walletUtils.broadcastBridgeAction(GOV1ADDR, {
method: 'tryExitOffer',
offerId,
});

const after = await getBalances([GOV1ADDR], 'uist');
t.log('uist balance after:', after);
const istBalanceAfter = await retryUntilCondition(
async () => getBalances([GOV1ADDR], 'uist'),
istBalance => istBalance > istBalanceBefore,
'tryExitOffer failed to reclaim stuck payment ',
{ setTimeout, retryIntervalMs: 5000, maxRetries: 15 },
);

t.true(
after > before,
istBalanceAfter > istBalanceBefore,
'The IST balance should increase after reclaiming the stuck payment',
);
});
Expand Down Expand Up @@ -98,7 +110,6 @@ test.serial(`ante handler sends fee only to vbank/reserve`, async t => {
{ chainId: CHAINID, from: GOV1ADDR, yes: true },
);

await waitForBlock();
t.like(result, { code: 0 });

const [feeCollectorEndBalances, vbankReserveEndBalances] = await getBalances([
Expand Down

0 comments on commit 8943c95

Please sign in to comment.