Skip to content

Commit

Permalink
fix(pg): check mismatch between plugins package and contracts library…
Browse files Browse the repository at this point in the history
… early
  • Loading branch information
sam-goldman committed Feb 10, 2024
1 parent 3010ff9 commit f5fac9c
Show file tree
Hide file tree
Showing 7 changed files with 111 additions and 51 deletions.
6 changes: 6 additions & 0 deletions .changeset/warm-ties-unite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
'@sphinx-labs/contracts': minor
'@sphinx-labs/plugins': minor
---

Check mismatch between plugins package and contracts library early
26 changes: 19 additions & 7 deletions packages/contracts/contracts/foundry/Sphinx.sol
Original file line number Diff line number Diff line change
Expand Up @@ -96,16 +96,28 @@ abstract contract Sphinx {
vm.makePersistent(address(sphinxUtils));
}

function sphinxCheckFork() external returns (bool forkInstalled) {
/**
* @notice Validates the user's Sphinx dependencies. Must be backwards compatible with previous
* versions of the Sphinx plugin package and the Sphinx contracts library. Specifically:
* - The function name must stay the same.
* - There must be no input parameters.
* - The returned values must not be removed or changed. However, new return values can
* be added.
*/
function sphinxValidate() external returns (string memory libraryVersion, bool forkInstalled) {
libraryVersion = sphinxUtils.sphinxLibraryVersion();

// Check that the Sphinx Foundry fork is installed.
vm.startStateDiffRecording();
new SphinxForkCheck{ salt: 0 }();
Vm.AccountAccess[] memory accountAccesses = vm.stopAndReturnStateDiff();
return
sphinxUtils.checkAccesses(
accountAccesses,
keccak256(type(SphinxForkCheck).creationCode),
keccak256(type(SphinxForkCheck).runtimeCode)
);
forkInstalled = sphinxUtils.checkAccesses(
accountAccesses,
keccak256(type(SphinxForkCheck).creationCode),
keccak256(type(SphinxForkCheck).runtimeCode)
);

return (libraryVersion, forkInstalled);
}

function sphinxCollectProposal(string memory _deploymentInfoPath) external {
Expand Down
80 changes: 61 additions & 19 deletions packages/contracts/test/SphinxInitCode.sol

Large diffs are not rendered by default.

7 changes: 2 additions & 5 deletions packages/plugins/src/cli/deploy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ import { SphinxMerkleTree, makeSphinxMerkleTree } from '@sphinx-labs/contracts'

import {
assertNoLinkedLibraries,
assertSphinxFoundryForkInstalled,
assertValidVersions,
compile,
getInitCodeWithArgsArray,
readInterface,
Expand All @@ -47,7 +47,6 @@ import { getFoundryToml } from '../foundry/options'
import { decodeDeploymentInfo, makeParsedConfig } from '../foundry/decode'
import { simulate } from '../hardhat/simulate'
import { SphinxContext } from './context'
import { checkLibraryVersion } from './utils'

export interface DeployArgs {
scriptPath: string
Expand Down Expand Up @@ -105,7 +104,7 @@ export const deploy = async (
etherscan,
} = foundryToml

await assertSphinxFoundryForkInstalled(scriptPath, targetContract)
await assertValidVersions(scriptPath, targetContract)

const forkUrl = rpcEndpoints[network]
if (!forkUrl) {
Expand Down Expand Up @@ -213,8 +212,6 @@ export const deploy = async (
sphinxPluginTypesInterface
)

checkLibraryVersion(deploymentInfo.sphinxLibraryVersion)

spinner.succeed(`Collected transactions.`)
spinner.start(`Building deployment...`)

Expand Down
7 changes: 2 additions & 5 deletions packages/plugins/src/cli/propose/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,13 +38,12 @@ import {
readInterface,
compile,
getInitCodeWithArgsArray,
assertSphinxFoundryForkInstalled,
assertValidVersions,
assertNoLinkedLibraries,
} from '../../foundry/utils'
import { SphinxContext } from '../context'
import { FoundryToml } from '../../foundry/types'
import { BuildParsedConfigArray } from '../types'
import { checkLibraryVersion } from '../utils'

/**
* @param isDryRun If true, the proposal will not be relayed to the back-end.
Expand Down Expand Up @@ -155,8 +154,6 @@ export const buildParsedConfigArray: BuildParsedConfigArray = async (
sphinxPluginTypesInterface
)

checkLibraryVersion(deploymentInfo.sphinxLibraryVersion)

collected.push({
deploymentInfo,
libraries: [], // We don't currently support linked libraries.
Expand Down Expand Up @@ -254,7 +251,7 @@ export const propose = async (

const foundryToml = await getFoundryToml()

await assertSphinxFoundryForkInstalled(scriptPath, targetContract)
await assertValidVersions(scriptPath, targetContract)

// We must load any ABIs after compiling the contracts to prevent a situation where the user
// clears their artifacts then calls this task, in which case the artifact won't exist yet.
Expand Down
10 changes: 0 additions & 10 deletions packages/plugins/src/cli/utils.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
import { CONTRACTS_LIBRARY_VERSION } from '@sphinx-labs/contracts'

export const BothNetworksSpecifiedError = `You must specify either 'testnets' or 'mainnets', but not both.`

export const ConfirmAndDryRunError =
Expand Down Expand Up @@ -29,11 +27,3 @@ export const coerceNetworks = (
// If none of the above conditions are met, throw a general error.
throw new Error(getInvalidNetworksArgumentError(arg))
}

export const checkLibraryVersion = (libraryVersion: string) => {
if (libraryVersion !== CONTRACTS_LIBRARY_VERSION) {
throw Error(
'Incorrect Sphinx library contracts version detected. Please run `npx sphinx install` to update to the required version.'
)
}
}
26 changes: 21 additions & 5 deletions packages/plugins/src/foundry/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ import {
recursivelyConvertResult,
getSystemContractInfo,
DETERMINISTIC_DEPLOYMENT_PROXY_ADDRESS,
CONTRACTS_LIBRARY_VERSION,
} from '@sphinx-labs/contracts'
import { ConstructorFragment, ethers } from 'ethers'
import { red } from 'chalk'
Expand Down Expand Up @@ -1352,16 +1353,31 @@ export const parseNestedContractDeployments = (
return { parsedContracts, unlabeled }
}

export const assertSphinxFoundryForkInstalled = async (
export const assertValidVersions = async (
scriptPath: string,
targetContract?: string
): Promise<void> => {
// Empirically check if our fork is functioning properly
const forkInstalled = await callForgeScriptFunction<{
// Validate the user's Sphinx dependencies. Specifically, we retrieve the Sphinx contracts library
// version and empirically check that our Foundry fork is functioning properly.
const output = await callForgeScriptFunction<{
libraryVersion: { value: string }
forkInstalled: { value: string }
}>(scriptPath, 'sphinxCheckFork()', [], undefined, targetContract)
}>(scriptPath, 'sphinxValidate()', [], undefined, targetContract)

const libraryVersion = output.returns.libraryVersion.value
// The raw string is wrapped in two sets of quotes, so we remove the outer quotes here.
.slice(1, -1)
const forkInstalled = output.returns.forkInstalled.value

if (libraryVersion !== CONTRACTS_LIBRARY_VERSION) {
throw Error(
`The version of the Sphinx library contracts does not match the Sphinx plugin version. Please\n` +
`update the library contracts by running the command:\n` +
`npx sphinx install`
)
}

if (forkInstalled.returns.forkInstalled.value === 'false') {
if (forkInstalled === 'false') {
throw new Error(
`Detected invalid Foundry version. Please use Sphinx's fork of Foundry by\n` +
`running the command:\n` +
Expand Down

0 comments on commit f5fac9c

Please sign in to comment.