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

build proposals for PriceFeed coreEvals on multiple platforms. #10146

Merged
merged 10 commits into from
Oct 10, 2024

Conversation

Chris-Hibbert
Copy link
Contributor

closes: #10076
refs: #10074

Description

Build platform-dependent proposals for the PriceFeed coreEval.

For each platform (not counting A3P) defined in updatePriceFeeds.js, a command is added, which invokes scripts/build-submission.sh three times to build each of the required proposals (updatePriceFeeds.js, add-auction.js, and upgradeVaults.js). The invocation for updatePriceFeeds takes a parameter specifying the platform.

Security Considerations

N/A

Scaling Considerations

N/A

Documentation Considerations

Not user-visible

Testing Considerations

test on DevNet and MainFork

Upgrade Considerations

It's all about upgrade. This coreEval has code that varies depending on the collaterals and oracles present, so we have to build separate proposals for each platform.

@Chris-Hibbert Chris-Hibbert added deployment Chain deployment mechanism (e.g. testnet) contract-upgrade oracle Related to on-chain oracles. labels Sep 24, 2024
@Chris-Hibbert Chris-Hibbert self-assigned this Sep 24, 2024
@Chris-Hibbert Chris-Hibbert requested a review from dckc September 27, 2024 17:42
@dckc
Copy link
Member

dckc commented Sep 27, 2024

Are these included in the artifacts produced in ci as part of #9929 ?

Looks like test-docker-build was Skipped. Want to add force-integration? Should I?

@Chris-Hibbert
Copy link
Contributor Author

Are these included in the artifacts produced in ci as part of #9929 ?

No, but I can add that.

Looks like test-docker-build was Skipped. Want to add force-integration? Should I?

This doesn't change anything that would effect that, I think. Why would that be helpful?

Copy link

cloudflare-workers-and-pages bot commented Sep 27, 2024

Deploying agoric-sdk with  Cloudflare Pages  Cloudflare Pages

Latest commit: 77a1aff
Status: ✅  Deploy successful!
Preview URL: https://207d4dc4.agoric-sdk.pages.dev
Branch Preview URL: https://10076-buildproposals.agoric-sdk.pages.dev

View logs

@Chris-Hibbert Chris-Hibbert added the force:integration Force integration tests to run on PR label Sep 27, 2024
@dckc
Copy link
Member

dckc commented Sep 27, 2024

based on ci gripes, it looks like we need to .gitignore the build artifacts:

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	a3p-integration/proposals/f:replace-price-feeds/devNet/
	a3p-integration/proposals/f:replace-price-feeds/mainNet/

nothing added to commit but untracked files present (use "git add" to track)
+ echo 'Unexpected dirty git status in Agoric SDK path'
+ exit 1
Unexpected dirty git status in Agoric SDK path

@Chris-Hibbert
Copy link
Contributor Author

based on ci gripes, it looks like we need to .gitignore the build artifacts

Okay, that took a couple of tries, but it's not complaining now.

@dckc
Copy link
Member

dckc commented Sep 30, 2024

I grabbed the core-eval-scripts.zip from the ci job. I see just 1 gov-price-feeds.js. Is that by design?

Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

as we discussed, gov-price-feeds-${config} should help

@Chris-Hibbert
Copy link
Contributor Author

The strings in the package.json have changed. See this comment.

@Chris-Hibbert Chris-Hibbert force-pushed the 9584-upgradePriceFeeds branch from bf1bd0c to eb941d4 Compare October 9, 2024 17:34
@Chris-Hibbert Chris-Hibbert requested a review from a team as a code owner October 9, 2024 17:34
Base automatically changed from 9584-upgradePriceFeeds to master October 9, 2024 18:14
Copy link
Member

@dckc dckc left a comment

Choose a reason for hiding this comment

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

It's not doing what I would expect.

@@ -4,9 +4,11 @@
"fromTag": "use-vaults-auctions"
},
"scripts": {
"build": "yarn run build:sdk && yarn run build:submissions && yarn run build:synthetic-chain",
"build": "yarn run build:sdk && yarn run build:submissions && yarn run build:synthetic-chain && yarn run build:priceFeeds-for-mainnet && yarn run build:priceFeeds-for-devnet",
Copy link
Member

Choose a reason for hiding this comment

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

I see only the 1 variant in the ci job Artifact:

$ unzip -l core-eval-scripts.zip |grep gov-price
     1024  2024-10-10 01:41   f-replace-price-feeds/gov-price-feeds-A3P_INTEGRATION-permit.json
     1463  2024-10-10 01:41   f-replace-price-feeds/gov-price-feeds-A3P_INTEGRATION-plan.json
     7069  2024-10-10 01:41   f-replace-price-feeds/gov-price-feeds-A3P_INTEGRATION.js

Is build:submissions the relevant thing to tweak rather than build?

@@ -6,7 +6,9 @@
"scripts": {
"build": "yarn run build:sdk && yarn run build:submissions && yarn run build:synthetic-chain",
"build:sdk": "make -C ../packages/deployment docker-build-sdk",
"build:submissions": "scripts/build-all-submissions.sh",
"build:submissions": "scripts/build-all-submissions.sh && yarn run build:priceFeeds-for-mainnet && yarn run build:priceFeeds-for-devnet",
Copy link
Member

Choose a reason for hiding this comment

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

seems to work!

$ unzip -l core-eval-scripts.zip |grep gov-price
     1436  2024-10-10 21:15   f-replace-price-feeds/devnet/gov-price-feeds-devnet-plan.json
     1024  2024-10-10 21:15   f-replace-price-feeds/devnet/gov-price-feeds-devnet-permit.json
     7195  2024-10-10 21:15   f-replace-price-feeds/devnet/gov-price-feeds-devnet.js
     1024  2024-10-10 21:15   f-replace-price-feeds/gov-price-feeds-A3P_INTEGRATION-permit.json
     1463  2024-10-10 21:15   f-replace-price-feeds/gov-price-feeds-A3P_INTEGRATION-plan.json
     7069  2024-10-10 21:15   f-replace-price-feeds/gov-price-feeds-A3P_INTEGRATION.js
     1024  2024-10-10 21:15   f-replace-price-feeds/main/gov-price-feeds-main-permit.json
     1430  2024-10-10 21:15   f-replace-price-feeds/main/gov-price-feeds-main-plan.json
     7284  2024-10-10 21:15   f-replace-price-feeds/main/gov-price-feeds-main.js

@Chris-Hibbert Chris-Hibbert added the automerge:squash Automatically squash merge label Oct 10, 2024
@mergify mergify bot merged commit d0a9e65 into master Oct 10, 2024
80 checks passed
@mergify mergify bot deleted the 10076-buildProposals branch October 10, 2024 21:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge:squash Automatically squash merge contract-upgrade deployment Chain deployment mechanism (e.g. testnet) force:integration Force integration tests to run on PR oracle Related to on-chain oracles.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

build separate priceFeed coreEval for each target environment
2 participants