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

feat: doctor checks for "set -e" in test scripts #207

Merged
merged 13 commits into from
Dec 10, 2024
3 changes: 2 additions & 1 deletion .github/pull_request_template.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ and then fulfill the remaining checklist items before merging.
Closes #

# adopt a passed proposal

Copy link
Member

Choose a reason for hiding this comment

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

is the prettier style set up to stick?

or was this just a 1-time?

Copy link
Member Author

@turadg turadg Dec 9, 2024

Choose a reason for hiding this comment

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

thanks for inquiring. It's one time and I didn't know why CI hadn't errored on it. Looks like --write makes --check only warn because it's updating: https://github.com/Agoric/agoric-3-proposals/actions/runs/12125081638/job/33804228478

I'll fix it

When a proposal passes on agoric-3 Mainnet, it should be included in the history that this synthetic image tracks.

- [ ] before this PR, change any `fromTag` using `latest` (such as [a3p-integration](https://github.com/Agoric/agoric-sdk/blob/master/a3p-integration/package.json)) to use a fixed version (otherwise they will fail when this PR changes latest and they pick it up)
Expand All @@ -19,11 +20,11 @@ When a proposal passes on agoric-3 Mainnet, it should be included in the history
This should not affect other repos but be prepared that it might.

## @agoric/synthetic-chain library

These need to be published to be consumed in other apps. It's not yet on 1.0 so breaking changes are allowed, but please don't make them gratuitously.

- [ ] indicate in the PR description when you expect these to be published and by whom (it's currently a manual process)

## tooling improvements

As long as it passes CI it should be fine.

8 changes: 4 additions & 4 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -50,14 +50,14 @@ jobs:
corepack enable || sudo corepack enable
yarn install

- name: Format
run: yarn format --check
- name: Lint repo
run: yarn lint

- name: Typecheck
- name: Typecheck synthetic-chain
run: yarn tsc
working-directory: packages/synthetic-chain

- name: Test
- name: Test synthetic-chain
run: yarn test
working-directory: packages/synthetic-chain

Expand Down
1 change: 0 additions & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,4 +19,3 @@ Each proposal to Mainnet (agoric-3) should be contributed to this repo before be
1. Move generally useful tools to the [`@agoric/synthetic-chain` package](https://github.com/Agoric/agoric-3-proposals/tree/main/packages/synthetic-chain).
2. Move tests that should continue to pass afterwards into [proposals/z:acceptance](https://github.com/Agoric/agoric-sdk/tree/master/a3p-integration/proposals/z%3Aacceptance).
3. Remove the passed proposal.

4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"build": "yarn build-cli && synthetic-chain build",
"doctor": "yarn synthetic-chain doctor",
"test": "yarn build-cli && synthetic-chain test",
"format": "prettier --write ."
"lint": "prettier --check ."
},
"dependencies": {
"@agoric/synthetic-chain": "workspace:*"
Expand All @@ -21,7 +21,7 @@
]
},
"license": "Apache-2.0",
"packageManager": "yarn@4.4.1",
"packageManager": "yarn@4.5.3",
"devDependencies": {
"prettier": "^3.3.3"
}
Expand Down
6 changes: 4 additions & 2 deletions packages/synthetic-chain/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,13 @@
"better-sqlite3": "^9.6.0",
"chalk": "^5.3.0",
"cosmjs-types": "^0.9.0",
"execa": "8.x"
"execa": "8.x",
"glob": "^11.0.0"
},
"devDependencies": {
"@agoric/cosmic-proto": "^0.4.1-dev-c5284e4.0",
"@agoric/cosmic-proto": "dev",
"@types/better-sqlite3": "^7.6.11",
"@types/glob": "^8.1.0",
"@types/node": "^18.19.50",
"ava": "^6.1.3",
"tsimp": "^2.0.11",
Expand Down
14 changes: 14 additions & 0 deletions packages/synthetic-chain/src/cli/doctor.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,25 @@
import fs from 'node:fs';
import path from 'node:path';
import { glob } from 'glob';
import { ProposalInfo } from './proposals.js';
import assert from 'node:assert';
import { execSync } from 'node:child_process';

const checkShellScripts = (proposalPath: string) => {
const shellScripts = glob.sync('*.sh', { cwd: proposalPath });
for (const script of shellScripts) {
const scriptPath = path.join(proposalPath, script);
const content = fs.readFileSync(scriptPath, 'utf-8');
assert(
content.includes('set -e'),
`${script} must include "set -e"; otherwise lines may fail silently. "set -euo pipefail" is recommended.`,
);
}
};

const fixupProposal = (proposal: ProposalInfo) => {
const proposalPath = path.join('proposals', proposal.path);
checkShellScripts(proposalPath);
const packageJson = JSON.parse(
fs.readFileSync(path.join(proposalPath, 'package.json'), 'utf-8'),
);
Expand Down
6 changes: 2 additions & 4 deletions packages/synthetic-chain/src/lib/econHelpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,10 +205,8 @@ export const pushPrices = async (price, brandIn, oraclesByBrand, round) => {
};

export const getRoundId = async (price, io = {}) => {
const {
agoric = { follow: agoricAmbient.follow },
prefix = 'published.',
} = io;
const { agoric = { follow: agoricAmbient.follow }, prefix = 'published.' } =
io;
const path = `:${prefix}priceFeed.${price}-USD_price_feed.latestRound`;
const round = await agoric.follow('-lF', path);
return parseInt(round.roundId);
Expand Down
2 changes: 1 addition & 1 deletion packages/synthetic-chain/src/lib/vstorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ harden(isStreamCell);
/**
* Extract one value from the vstorage stream cell in a QueryDataResponse
*
* @param {import('@agoric/cosmic-proto/dist/codegen/agoric/vstorage/query.js').QueryDataResponse} data
* @param {import('@agoric/cosmic-proto/vstorage/query.js').QueryDataResponse} data
* @param {number} [index] index of the desired value in a deserialized stream cell
*/
export const extractStreamCellValue = (data, index = -1) => {
Expand Down
5 changes: 3 additions & 2 deletions packages/synthetic-chain/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,12 @@
"allowSyntheticDefaultImports": true,
"allowJs": true,
"checkJs": true,
"skipLibCheck": true,
"strict": true,
"noImplicitAny": false,
"maxNodeModuleJsDepth": 2,
"module": "ES2022",
"moduleResolution": "Node",
"module": "NodeNext",
"moduleResolution": "nodenext",
"noEmit": true,
"target": "ESNext"
},
Expand Down
1 change: 0 additions & 1 deletion proposals/34:upgrade-10/performActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
getUser,
newOfferId,
waitForBlock,

openVault,
adjustVault,
closeVault,
Expand Down
1 change: 0 additions & 1 deletion proposals/34:upgrade-10/post.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import {
GOV2ADDR,
GOV3ADDR,
USER1ADDR,

agd,
agoric,
calculateWalletState,
Expand Down
49 changes: 24 additions & 25 deletions proposals/59:usdc-psm/submission/add-usdc.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,17 @@

const manifestBundleRef = {
bundleID:
"b1-4c34c89b707bc8ece5a41e97e6a354081f7ae8a40391f1462848348613dd1218dcce574b3e30901a9825a966cb85bda6a92ba9f9ce9ba325e4c475f9a678b930",
'b1-4c34c89b707bc8ece5a41e97e6a354081f7ae8a40391f1462848348613dd1218dcce574b3e30901a9825a966cb85bda6a92ba9f9ce9ba325e4c475f9a678b930',
};
const getManifestCall = harden([
"getManifestForPsm",
'getManifestForPsm',
{
anchorOptions: {
decimalPlaces: 6,
denom:
"ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9",
keyword: "USDC",
proposedName: "USDC",
'ibc/FE98AAD68F02F03565E9FA39A5E627946699B2B07115889ED812D8BA639576A9',
keyword: 'USDC',
proposedName: 'USDC',
},
installKeys: {},
},
Expand All @@ -24,12 +24,12 @@ const overrideManifest = {
agoricNamesAdmin: true,
anchorBalancePayments: true,
anchorKits: true,
bankManager: "bank",
bankManager: 'bank',
startUpgradable: true,
},
installation: {
consume: {
mintHolder: "zoe",
mintHolder: 'zoe',
},
},
produce: {
Expand All @@ -44,36 +44,36 @@ const overrideManifest = {
startPSM: {
brand: {
consume: {
IST: "zoe",
IST: 'zoe',
},
},
consume: {
agoricNamesAdmin: true,
anchorBalancePayments: true,
board: true,
chainStorage: true,
chainTimerService: "timer",
chainTimerService: 'timer',
diagnostics: true,
econCharterKit: "econCommitteeCharter",
economicCommitteeCreatorFacet: "economicCommittee",
feeMintAccess: "zoe",
econCharterKit: 'econCommitteeCharter',
economicCommitteeCreatorFacet: 'economicCommittee',
feeMintAccess: 'zoe',
provisionPoolStartResult: true,
psmKit: true,
zoe: "zoe",
zoe: 'zoe',
},
installation: {
consume: {
contractGovernor: "zoe",
psm: "zoe",
contractGovernor: 'zoe',
psm: 'zoe',
},
},
instance: {
consume: {
economicCommittee: "economicCommittee",
economicCommittee: 'economicCommittee',
},
},
produce: {
psmKit: "true",
psmKit: 'true',
},
vatParameters: {
chainStorageEntries: true,
Expand All @@ -93,21 +93,21 @@ const overrideManifest = {
const { entries, fromEntries } = Object;

// deeplyFulfilled is a bit overkill for what we need.
const shallowlyFulfilled = async (obj) => {
const shallowlyFulfilled = async obj => {
if (!obj) {
return obj;
}
const ents = await Promise.all(
entries(obj).map(async ([key, valueP]) => {
const value = await valueP;
return [key, value];
})
}),
);
return fromEntries(ents);
};

/** @param {ChainBootstrapSpace & BootstrapPowers & { evaluateBundleCap: any }} allPowers */
const behavior = async (allPowers) => {
const behavior = async allPowers => {
// NOTE: If updating any of these names extracted from `allPowers`, you must
// change `permits` above to reflect their accessibility.
const {
Expand All @@ -120,13 +120,13 @@ const overrideManifest = {
const [exportedGetManifest, ...manifestArgs] = getManifestCall;

// Get the on-chain installation containing the manifest and behaviors.
console.info("evaluateBundleCap", {
console.info('evaluateBundleCap', {
manifestBundleRef,
exportedGetManifest,
vatAdminSvc,
});
let bcapP;
if ("bundleName" in manifestBundleRef) {
if ('bundleName' in manifestBundleRef) {
bcapP = E(vatAdminSvc).getNamedBundleCap(manifestBundleRef.bundleName);
} else {
bcapP = E(vatAdminSvc).getBundleCap(manifestBundleRef.bundleID);
Expand All @@ -135,7 +135,7 @@ const overrideManifest = {

const manifestNS = await evaluateBundleCap(bundleCap);

console.error("execute", {
console.error('execute', {
exportedGetManifest,
behaviors: Object.keys(manifestNS),
});
Expand All @@ -153,7 +153,7 @@ const overrideManifest = {
behaviors: manifestNS,
manifest: overrideManifest || manifest,
makeConfig: (name, _permit) => {
log("coreProposal:", name);
log('coreProposal:', name);
return { options };
},
});
Expand All @@ -162,4 +162,3 @@ const overrideManifest = {
// Make the behavior the completion value.
return behavior;
})({ manifestBundleRef, getManifestCall, overrideManifest, E });

1 change: 1 addition & 0 deletions proposals/71:upgrade-14/use.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/bin/bash
set -e

# Place here any actions that should happen after the upgrade has executed. The
# actions are executed in the upgraded chain software and the effects are
Expand Down
8 changes: 4 additions & 4 deletions proposals/74:upgrade-15/prepare.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash

# Exit when any command fails
set -uxeo pipefail
set -euxo pipefail

# Place here any actions that should happen before the upgrade is proposed. The
# actions are executed in the previous chain software, and the effects are
Expand All @@ -10,16 +10,16 @@ set -uxeo pipefail

printISTBalance() {
addr=$(agd keys show -a "$1" --keyring-backend=test)
agd query bank balances "$addr" -o json \
| jq -c '.balances[] | select(.denom=="uist")'
agd query bank balances "$addr" -o json |
jq -c '.balances[] | select(.denom=="uist")'

}

echo TEST: Offer with bad invitation
printISTBalance gov1

badInvitationOffer=$(mktemp)
cat > "$badInvitationOffer" << 'EOF'
cat >"$badInvitationOffer" <<'EOF'
{"body":"#{\"method\":\"executeOffer\",\"offer\":{\"id\":\"bad-invitation-15\",\"invitationSpec\":{\"callPipe\":[[\"badMethodName\"]],\"instancePath\":[\"reserve\"],\"source\":\"agoricContract\"},\"proposal\":{\"give\":{\"Collateral\":{\"brand\":\"$0.Alleged: IST brand\",\"value\":\"+15000\"}}}}}","slots":["board0257"]}
EOF

Expand Down
2 changes: 1 addition & 1 deletion proposals/75:upgrade-16/prepare.sh
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#!/bin/bash

# Exit when any command fails
set -uxeo pipefail
set -euxo pipefail

# Place here any actions that should happen before the upgrade is proposed. The
# actions are executed in the previous chain software, and the effects are
Expand Down
1 change: 1 addition & 0 deletions proposals/76:vaults-auctions/eval.sh
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#!/bin/bash
set -e

echo "[$PROPOSAL] Recording the auctioneer instance"
./saveAuctionInstance.js
Expand Down
11 changes: 7 additions & 4 deletions proposals/76:vaults-auctions/submission/add-auction.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,16 @@
// This is generated by writeCoreEval; please edit!
/* eslint-disable */

const manifestBundleRef = {bundleID:"b1-47c15fc48569fde3afe4e4947f877242d2d6367c07876951acea99a20d9c890974f3d237f22b5033a84c5e3d506acc6e899e519590a8557d49d6d43611dc9c65"};
const manifestBundleRef = {
bundleID:
'b1-47c15fc48569fde3afe4e4947f877242d2d6367c07876951acea99a20d9c890974f3d237f22b5033a84c5e3d506acc6e899e519590a8557d49d6d43611dc9c65',
};
const getManifestCall = harden([
"getManifestForAddAuction",
'getManifestForAddAuction',
{
auctionsRef: {
bundleID: "b1-31bf1ef20dd190a9f541471bc15238a51f621ff2340e6eb225214b9fdf3970f2bc3bc4fe151a79ef2e740b2679cf03f553b89a828da704dec9ccba9463fc3f79",
bundleID:
'b1-31bf1ef20dd190a9f541471bc15238a51f621ff2340e6eb225214b9fdf3970f2bc3bc4fe151a79ef2e740b2679cf03f553b89a828da704dec9ccba9463fc3f79',
},
},
]);
Expand Down Expand Up @@ -195,4 +199,3 @@ const behavior = (({
return coreProposalBehavior;
})({ manifestBundleRef, getManifestCall, customManifest, E });
behavior;

Loading
Loading