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

Scripts and unit tests for contract upgrade and ownership transfer #89

Merged
merged 15 commits into from
Jan 16, 2025

Conversation

aarshkshah1992
Copy link
Collaborator

@aarshkshah1992 aarshkshah1992 commented Jan 8, 2025

@ZenGround0

Manually tested

  • Upgrading the contract
  • Transferring ownership of the contract
  • Upgrading the contract such that no further upgrades are possible:
    • This can be done by making the _authorizeUpgrade function in the upgradedPDPVerifier contract always revert.

This PR has unit tests for testing contract upgrade and ownership transfer and also scripts to do it all on calibnet.

@aarshkshah1992
Copy link
Collaborator Author

I am now testing ownership/upgrade with a multisig.

@ZenGround0 ZenGround0 force-pushed the feat/upgrade-test-scripts branch from 2907cca to f487ecf Compare January 13, 2025 03:32
#####################################

: "${FIL_CALIBNET_RPC_URL:?FIL_CALIBNET_RPC_URL not set. Please export it and rerun.}"
: "${FIL_CALIBNET_PRIVATE_KEY:?FIL_CALIBNET_PRIVATE_KEY not set. Please export it and rerun.}"
Copy link
Contributor

@ZenGround0 ZenGround0 Jan 13, 2025

Choose a reason for hiding this comment

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

Purely out of curiosity how do you get your private keys? In the past I've ended up needing to write my own script to extract from eth keystore since that design works hard to avoid extraction.

)

# Extract the deployed address from JSON output
PDP_VERIFIER_ADDRESS=$(echo "$DEPLOY_OUTPUT_VERIFIER" | jq -r '.deployedTo')
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm having trouble parsing this on both OSX and linux. There is no deployedTo field in my output json. Have you gotten it working since making this change?

@ZenGround0 ZenGround0 force-pushed the feat/upgrade-test-scripts branch from 09ba0a4 to ccab75e Compare January 13, 2025 06:51
Copy link
Contributor

@ZenGround0 ZenGround0 left a comment

Choose a reason for hiding this comment

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

Working!

@ZenGround0 ZenGround0 merged commit 51ba429 into main Jan 16, 2025
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants