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: setup for running checks locally #29

Merged
merged 4 commits into from
Oct 1, 2021
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .github/workflows/governance-checks.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ jobs:
env:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
ETHERSCAN_API_KEY: ${{ secrets.ETHERSCAN_API_KEY }}
INFURA_API_KEY: ${{ secrets.INFURA_API_KEY }}
RPC_URL: ${{ secrets.RPC_URL }}
DAO_NAME: ${{ matrix.DAO_NAME }}
GOVERNOR_ADDRESS: ${{ matrix.GOVERNOR_ADDRESS }}
RUNNING_LOCALLY: 0
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
node_modules
.idea

reports/
Copy link
Contributor

Choose a reason for hiding this comment

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

add .env.local maybe

Copy link
Contributor Author

@mds1 mds1 Sep 30, 2021

Choose a reason for hiding this comment

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

Good call, must have been using my global gitignore. Done in 6bd5777

27 changes: 25 additions & 2 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,29 @@ including automated scripts that apply checks to live proposals to allow
for better informed voting.


# Reports
## Reports

Find the reports [here](https://github.com/Uniswap/governance-seatbelt/tree/reports)
Find the reports [here](https://github.com/Uniswap/governance-seatbelt/tree/reports) when run in CI, or in the `reports` folder if running locally

## Development

Create a file called `.env` with the following environment variables:

```sh
# URL to your node, e.g. Infura or Alchemy endpoint
RPC_URL=yourNodeUrl

# Etherscan API key: https://etherscan.io/myapikey
ETHERSCAN_API_KEY=yourEtherscanApiKey

# Name of the DAO to check
DAO_NAME=Compound

# Address of that DAO's governor contract
GOVERNOR_ADDRESS=0xc0Da02939E1441F497fd74F78cE7Decb17B66529

# Set to 1 if running locally, or 0 for CI
RUNNING_LOCALLY=1
```

Other environment variables needed for CI can be found in `.github/workflows/governance-checks.yaml`
66 changes: 39 additions & 27 deletions index.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
require('dotenv').config()
import fs from 'fs';
import {
DAO_NAME,
GITHUB_REPO_NAME,
GITHUB_REPO_OWNER,
GOVERNOR_ADDRESS,
REPORTS_BRANCH,
RUNNING_LOCALLY
} from "./utils/constants";
import { github } from "./utils/clients/github";
import { governorBravo } from "./utils/contracts/governor-bravo";
import { provider } from "./utils/clients/ethers";
import { AllCheckResults, Proposal } from "./types";
Expand All @@ -32,6 +34,7 @@ async function main() {
.map((proposal) => proposal.args as unknown as Proposal);

for (const proposal of activeProposals) {
console.log(`Checking proposal ${proposal.id}...`);
const checkResults: AllCheckResults = Object.fromEntries(
await Promise.all(
Object.keys(ALL_CHECKS).map(async (checkId) => [
Expand All @@ -56,37 +59,46 @@ async function main() {
: null,
]);

let sha: string | undefined;
try {
const { data } = await github.rest.repos.getContent({
const report = toProposalReport(
{ start: startBlock, end: endBlock, current: latestBlock },
proposal,
checkResults
);

if (RUNNING_LOCALLY) {
// Running locally, dump to file
const dir = `./reports/${DAO_NAME}/${GOVERNOR_ADDRESS}/`; // TODO more robust way to keep this in sync with `path`
if (!fs.existsSync(dir)) fs.mkdirSync(dir, { recursive: true });
fs.writeFileSync(`${dir}/${proposal.id}.md`, report);

} else {
// Running in CI, save to file on REPORTS_BRANCH
const { github } = await import("./utils/clients/github"); // lazy load to avoid errors about missing env vars when not in CI
Copy link
Contributor

Choose a reason for hiding this comment

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

i would probably do this slightly differently, where you just have a different entry point for main and local testing, but you can probably also just use something like is the github token to enter this code path

also would be cool if our testing was scripted (integration testing) for ongoing maintenance, but that's a lot of work

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i would probably do this slightly differently, where you just have a different entry point for main and local testing, but you can probably also just use something like is the github token to enter this code path

Yea different entry point could work also, but probably would want to pull out some of the existing code into standalone methods to avoid duplicating code. My thinking was this is the shortest path forward to knocking out the bigger unknowns (e.g. tx simulation), then could refactor later

use something like is the github token to enter this code path

Do you mean remove the RUNNING_LOCALLY flag and use the presence of the GitHub API keys in its place? Can do that, but do you know what env vars to check for? new Octokit() threw with a missing GITHUB_ACTION env var and I didn't see that defined anywhere in the CI config, so that was confusing

also would be cool if our testing was scripted (integration testing) for ongoing maintenance, but that's a lot of work

Could you create an issue with some more details on what you had in mind here?

Copy link
Contributor

@moodysalem moodysalem Sep 30, 2021

Choose a reason for hiding this comment

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

Yea different entry point could work also, but probably would want to pull out some of the existing code into standalone methods to avoid duplicating code. My thinking was this is the shortest path forward to knocking out the bigger unknowns (e.g. tx simulation), then could refactor later

agree, you would want both entry points to call into the report generator code, where the cli entry point writes the output to console, (or somewhere useful for manual testing), takes cli arguments for which governor and/or proposal to check, etc.
and the ci entry point saves report results to the repo

it shouldn't be a large refactor, but i'm ok with doing it later

Do you mean remove the RUNNING_LOCALLY flag and use the presence of the GitHub API keys in its place?

yeah, you only need to check the one GITHUB_TOKEN i think

created #31

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Cool that all makes sense, thanks 👌

let sha: string | undefined;
try {
const { data } = await github.rest.repos.getContent({
owner: GITHUB_REPO_OWNER,
repo: GITHUB_REPO_NAME,
ref: REPORTS_BRANCH,
path,
});
if ("sha" in data) {
sha = data.sha;
}
} catch (error) {
console.warn("Failed to get sha for file at path", path, error);
}

await github.rest.repos.createOrUpdateFileContents({
owner: GITHUB_REPO_OWNER,
repo: GITHUB_REPO_NAME,
ref: REPORTS_BRANCH,
branch: REPORTS_BRANCH,
message: `Update ${path} as of ${formattedDateTime}`,
content: Buffer.from(report, "utf-8").toString("base64"),
path,
sha,
});
if ("sha" in data) {
sha = data.sha;
}
} catch (error) {
console.warn("Failed to get sha for file at path", path, error);
}

await github.rest.repos.createOrUpdateFileContents({
owner: GITHUB_REPO_OWNER,
repo: GITHUB_REPO_NAME,
branch: REPORTS_BRANCH,
message: `Update ${path} as of ${formattedDateTime}`,
content: Buffer.from(
toProposalReport(
{ start: startBlock, end: endBlock, current: latestBlock },
proposal,
checkResults
),
"utf-8"
).toString("base64"),
path,
sha,
});
} catch (error) {
console.error("Failed to update file contents", error);
}
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
"@ethercast/etherscan-client": "^1.0.1",
"@octokit/action": "^3.15.2",
"@types/node": "^16.7.13",
"dotenv": "^10.0.0",
"ethers": "^5.4.6",
"ts-node": "^10.2.1",
"typescript": "^4.4.2"
Expand Down
4 changes: 2 additions & 2 deletions utils/clients/ethers.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { providers } from "ethers";
import { INFURA_API_KEY } from "../constants";
import { RPC_URL } from "../constants";

export const provider = new providers.InfuraProvider(1, INFURA_API_KEY);
export const provider = new providers.JsonRpcProvider(RPC_URL);
3 changes: 2 additions & 1 deletion utils/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ export const REPOSITORY: string = process.env.GITHUB_REPOSITORY!;
export const [GITHUB_REPO_OWNER, GITHUB_REPO_NAME] =
typeof REPOSITORY === "string" ? REPOSITORY.split("/") : [];
export const GOVERNOR_ADDRESS: string = process.env.GOVERNOR_ADDRESS!;
export const INFURA_API_KEY: string = process.env.INFURA_API_KEY!;
export const RPC_URL: string = process.env.RPC_URL!;
export const ETHERSCAN_API_KEY: string = process.env.ETHERSCAN_API_KEY!;
export const DAO_NAME: string = process.env.DAO_NAME!;
export const REPORTS_BRANCH = "reports";
export const RUNNING_LOCALLY = Boolean(process.env.RUNNING_LOCALLY) // use 1 or 0 because Boolean('false') = true
5 changes: 5 additions & 0 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -578,6 +578,11 @@ diff@^4.0.1:
resolved "https://registry.yarnpkg.com/diff/-/diff-4.0.2.tgz#60f3aecb89d5fae520c11aa19efc2bb982aade7d"
integrity sha512-58lmxKSA4BNyLz+HHMUzlOEpg09FV+ev6ZMe3vJihgdxzgcwZ8VoEEPmALCZG9LmqfVoNMMKpttIYTVG6uDY7A==

dotenv@^10.0.0:
version "10.0.0"
resolved "https://registry.yarnpkg.com/dotenv/-/dotenv-10.0.0.tgz#3d4227b8fb95f81096cdd2b66653fb2c7085ba81"
integrity sha512-rlBi9d8jpv9Sf1klPjNfFAuWDjKLwTIJJ/VxtoTwIR6hnZxcEOQCZg2oIL3MWBYw5GpUDKOEnND7LXTbIpQ03Q==

[email protected]:
version "6.5.4"
resolved "https://registry.yarnpkg.com/elliptic/-/elliptic-6.5.4.tgz#da37cebd31e79a1367e941b592ed1fbebd58abbb"
Expand Down