Skip to content

Commit

Permalink
test(a3p): extend governance test coverage for acceptance proposal (#…
Browse files Browse the repository at this point in the history
…10555)

closes: https://github.com/Agoric/BytePitchPartnerEng/issues/24
closes: #10708

## Description


This PR has the objective of extending the a3p-integration acceptance proposal test coverage. More specifically, it intend to test that:
- Configured EC governed parameters are intact following the contract upgrade
- Governance proposals history is visible

The test plan to verify that the parameters governed by the economicCommittee are intact when the contract is upgraded is:
- Propose a parameter change
- Verify that the value was updated
- Execute a null upgrade to the respective contract
- Verify that the value kept the same

Although more contracts are governed by economicCommittee, the ones selected to be included in this test case were `vaultFactory` and `provisionPool`. A detailed discussion on the rationale behind this decision can be found at https://github.com/Agoric/BytePitchPartnerEng/issues/24

Note: The existing governance test and helper functions were refactored to allow the `makeGovernanceDriver` function to be contract agnostic, in order to be reused by vaultFactory, provisionPool and any other contract that is intend to be tested in the future.

The test plan is to verify that the vstorage node published.committees.Economic_Committee.latestQuestion displays the history of proposed Economic Committee parameter changes is:
- Use the readHistory method to fetch all parameter change proposals from latestQuestion node, starting from block height 0.
- Verify that returned value is not an empty list
- Construct a list of expected parameters changes proposed by the economic committee prior to the governance test.
- Compare the parameters keys of both lists match.

### Security Considerations


No security considerations.
### Scaling Considerations


The "Governance proposals history" test compares vstorage values against a hardcoded list, requiring manual updates to expectedParametersChanges for new proposals or tests that executes a EC governed param changes made prior to this test. 

To prevent the halting the test when the developer does not update the hardcoded list, this test is only asserting that the value returned from the published.committees.Economic_Committee.latestQuestion vstorage node is not empty.
Then it will compare both lists and if any mismatches is found, it will:
- print an error message notifying the mismatch. 
- print both current and expected changes proposals records

Note that this solution may risks oversight and could lead to latent issues, so I am open a implement a new design if desired.

### Documentation Considerations


We propose adding a table to the z:acceptance README file. This table will:
- List the parameters being modified during acceptance test execution.
- Identify where and why these changes occur, providing developers with a clear understanding of parameter dependencies.

An [issue](https://github.com/Agoric/BytePitchPartnerEng/issues/43) was created to address this task.

### Testing Considerations


The methods exported by `makeVstorageKit` at the `@agoric/client-utils` package, seems to display an unexpected behaviour, more specifically, when using the `readFully` method to query Vstorage node full history. 
As a workaround, I updated the `makeVStorage` function at `z:acceptance/test-lib/rpc.js` to follow the same implementation as the `multichain-testing/tools/batchQuery.js.`
The rational behind this decision is explained in detail on the issue #10574 

If desired, I can add these changes to `@client-utils/src/vstorage.js` in a future PR, contribute to the ongoing effort of reducing the dependency of the acceptance test-lib.

The order of execution of the acceptance tests had to be updated to make sure that the `governance.test.js` would run after the state-sync acceptance tests to prevent the behavior described here #10574 

### Upgrade Considerations


No upgrade considerations.
  • Loading branch information
mergify[bot] authored Dec 17, 2024
2 parents 9e124c3 + 28b4884 commit 9835fb0
Show file tree
Hide file tree
Showing 11 changed files with 792 additions and 84 deletions.
2 changes: 2 additions & 0 deletions a3p-integration/proposals/z:acceptance/.gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ restart-valueVow
start-valueVow
localchaintest-submission
recorded-instances-submission
upgrade-vaultFactory
upgrade-provisionPool
237 changes: 174 additions & 63 deletions a3p-integration/proposals/z:acceptance/governance.test.js
Original file line number Diff line number Diff line change
@@ -1,57 +1,35 @@
/* global fetch setTimeout */
/* global fetch */

import test from 'ava';

import { makeWalletUtils } from '@agoric/client-utils';
import { GOV1ADDR, GOV2ADDR } from '@agoric/synthetic-chain';
import { makeGovernanceDriver } from './test-lib/governance.js';
import { networkConfig } from './test-lib/index.js';
import { makeTimerUtils } from './test-lib/utils.js';
import { agdWalletUtils } from './test-lib/index.js';
import { upgradeContract } from './test-lib/utils.js';
import { networkConfig } from './test-lib/rpc.js';

const GOV4ADDR = 'agoric1c9gyu460lu70rtcdp95vummd6032psmpdx7wdy';
const governanceAddresses = [GOV4ADDR, GOV2ADDR, GOV1ADDR];

// TODO test-lib export `walletUtils` with this ambient authority like s:stake-bld has
/** @param {number} ms */
const delay = ms =>
new Promise(resolve => setTimeout(() => resolve(undefined), ms));
const { getLastUpdate, readLatestHead } = agdWalletUtils;
const governanceDriver = await makeGovernanceDriver(fetch, networkConfig);

const testSkipXXX = test.skip; // same lenth as test.serial to avoid reformatting all lines

// z:acceptance governance fails/flakes: No quorum #10708
testSkipXXX(
test.serial(
'economic committee can make governance proposal and vote on it',
async t => {
const { waitUntil } = makeTimerUtils({ setTimeout });
const { readLatestHead, getLastUpdate, getCurrentWalletRecord } =
await makeWalletUtils({ delay, fetch }, networkConfig);
const governanceDriver = await makeGovernanceDriver(fetch, networkConfig);

/** @type {any} */
const instance = await readLatestHead(`published.agoricNames.instance`);
const instances = Object.fromEntries(instance);

const wallet = await getCurrentWalletRecord(governanceAddresses[0]);
const usedInvitations = wallet.offerToUsedInvitation;

const charterInvitation = usedInvitations.find(
v =>
v[1].value[0].instance.getBoardId() ===
instances.econCommitteeCharter.getBoardId(),
);
assert(charterInvitation, 'missing charter invitation');

const params = {
ChargingPeriod: 400n,
};
const path = { paramPath: { key: 'governedParams' } };
t.log('Proposing param change', { params, path });
const instanceName = 'VaultFactory';

await governanceDriver.proposeVaultDirectorParamChange(
await governanceDriver.proposeParamChange(
governanceAddresses[0],
params,
path,
charterInvitation[0],
instanceName,
30,
);

const questionUpdate = await getLastUpdate(governanceAddresses[0]);
Expand All @@ -62,22 +40,9 @@ testSkipXXX(

t.log('Voting on param change');
for (const address of governanceAddresses) {
const voteWallet =
/** @type {import('@agoric/smart-wallet/src/smartWallet.js').CurrentWalletRecord} */ (
await readLatestHead(`published.wallet.${address}.current`)
);
const committeeInvitationForVoter =
await governanceDriver.getCommitteeInvitation(address);

const usedInvitationsForVoter = voteWallet.offerToUsedInvitation;

const committeeInvitationForVoter = usedInvitationsForVoter.find(
v =>
v[1].value[0].instance.getBoardId() ===
instances.economicCommittee.getBoardId(),
);
assert(
committeeInvitationForVoter,
`${address} must have committee invitation`,
);
await governanceDriver.voteOnProposedChanges(
address,
committeeInvitationForVoter[0],
Expand All @@ -90,22 +55,168 @@ testSkipXXX(
});
}

const latestQuestion =
/** @type {import('@agoric/governance/src/types.js').QuestionSpec} */ (
await readLatestHead(
'published.committees.Economic_Committee.latestQuestion',
)
await governanceDriver.waitForElection();
},
);

test.serial(
'VaultFactory governed parameters are intact following contract upgrade',
async t => {
/** @type {any} */
const vaultFactoryParamsBefore = await readLatestHead(
'published.vaultFactory.governance',
);

/*
* At the previous test ('economic committee can make governance proposal and vote on it')
* The value of ChargingPeriod was updated to 400
* The 'published.vaultFactory.governance' node should reflect that change.
*/
t.is(
vaultFactoryParamsBefore.current.ChargingPeriod.value,
400n,
'vaultFactory ChargingPeriod parameter value is not the expected ',
);

await upgradeContract('upgrade-vaultFactory', 'zcf-b1-6c08a-vaultFactory');

const vaultFactoryParamsAfter = await readLatestHead(
'published.vaultFactory.governance',
);

t.deepEqual(
vaultFactoryParamsAfter,
vaultFactoryParamsBefore,
'vaultFactory governed parameters did not match',
);
},
);

test.serial(
'economic committee can make governance proposal for ProvisionPool',
async t => {
/** @type {any} */
const brand = await readLatestHead(`published.agoricNames.brand`);
const brands = Object.fromEntries(brand);

const params = {
PerAccountInitialAmount: { brand: brands.IST, value: 100_000n },
};
const path = { paramPath: { key: 'governedParams' } };
const instanceName = 'provisionPool';

await governanceDriver.proposeParamChange(
governanceAddresses[0],
params,
path,
instanceName,
30,
);

const questionUpdate = await getLastUpdate(governanceAddresses[0]);
t.like(questionUpdate, {
status: { numWantsSatisfied: 1 },
});

for (const address of governanceAddresses) {
const committeeInvitationForVoter =
await governanceDriver.getCommitteeInvitation(address);

await governanceDriver.voteOnProposedChanges(
address,
committeeInvitationForVoter[0],
);
t.log('Waiting for deadline', latestQuestion);
/** @type {bigint} */
// @ts-expect-error assume POSIX seconds since epoch
const deadline = latestQuestion.closingRule.deadline;
await waitUntil(deadline);

const latestOutcome = await readLatestHead(
'published.committees.Economic_Committee.latestOutcome',

const voteUpdate = await getLastUpdate(address);
t.like(voteUpdate, {
status: { numWantsSatisfied: 1 },
});
}

await governanceDriver.waitForElection();
},
);

test.serial(
'ProvisionPool governed parameters are intact following contract upgrade',
async t => {
/** @type {any} */
const provisionPoolParamsBefore = await readLatestHead(
'published.provisionPool.governance',
);

/*
* At the previous test ('economic committee can make governance proposal and vote on it')
* The value of ChargingPeriod was updated to 400
* The 'published.vaultFactory.governance' node should reflect that change.
*/
t.is(
provisionPoolParamsBefore.current.PerAccountInitialAmount.value.value,
100_000n,
'provisionPool PerAccountInitialAmount parameter value is not the expected ',
);

await upgradeContract(
'upgrade-provisionPool',
'zcf-b1-db93f-provisionPool',
);

/** @type {any} */
const provisionPoolParamsAfter = await readLatestHead(
'published.provisionPool.governance',
);

t.deepEqual(
provisionPoolParamsAfter.current.PerAccountInitialAmount,
provisionPoolParamsBefore.current.PerAccountInitialAmount,
'provisionPool governed parameters did not match',
);
t.log('Verifying latest outcome', latestOutcome);
t.like(latestOutcome, { outcome: 'win' });
},
);

test.serial('Governance proposals history is visible', async t => {
/*
* List ordered from most recent to earliest of Economic Committee
* parameter changes proposed prior to the execution of this test.
*
* XXX a dynamic solution should replace this hardcoded list to ensure
* the acceptance tests scalability
*/
const expectedParametersChanges = [
['PerAccountInitialAmount'], // z:acceptance/governance.test.js
['ChargingPeriod'], // z:acceptance/governance.test.js
['DebtLimit'], // z:acceptance/vaults.test.js
['GiveMintedFee', 'MintLimit', 'WantMintedFee'], // z:acceptance/psm.test.js
['DebtLimit'], // z:acceptance/scripts/test-vaults.mts
['ClockStep', 'PriceLockPeriod', 'StartFrequency'], // z:acceptance/scripts/test-vaults.mts
['DebtLimit'], // agoric-3-proposals/proposals/34:upgrade-10/performActions.js
['ClockStep', 'PriceLockPeriod', 'StartFrequency'], // agoric-3-proposals/proposals/34:upgrade-10/performActions.js
];

// history of Economic Committee parameters changes proposed since block height 0
const history = await governanceDriver.getLatestQuestionHistory();
t.true(
history.length > 0,
'published.committees.Economic_Committee.latestQuestion node should not be empty',
);

const changedParameters = history.map(changes => Object.keys(changes));

/*
* In case you see the error message bellow and you
* executed an VoteOnParamChange offer prior to this test,
* please make sure to update the expectedParametersChanges list.
*/
if (
!(
JSON.stringify(changedParameters) ===
JSON.stringify(expectedParametersChanges)
)
) {
console.error(
`ERROR: Economic_Committee parameters changes history does not match with the expected list`,
);
t.log('Economic_Committee parameters changes history: ', changedParameters);
t.log('Expected parameters changes history: ', expectedParametersChanges);
}
});
4 changes: 3 additions & 1 deletion a3p-integration/proposals/z:acceptance/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
"testing/start-valueVow.js start-valueVow",
"testing/recorded-retired-instances.js recorded-instances-submission",
"vats/test-localchain.js localchaintest-submission",
"testing/restart-valueVow.js restart-valueVow"
"testing/restart-valueVow.js restart-valueVow",
"testing/upgrade-vaultFactory.js upgrade-vaultFactory",
"vats/upgrade-provisionPool.js upgrade-provisionPool"
]
},
"type": "module",
Expand Down
Loading

0 comments on commit 9835fb0

Please sign in to comment.