Skip to content

Commit

Permalink
feat(core): add support for multiple project owners
Browse files Browse the repository at this point in the history
  • Loading branch information
sam-goldman committed Aug 14, 2023
1 parent f4835f1 commit eb0cc1c
Show file tree
Hide file tree
Showing 14 changed files with 634 additions and 516 deletions.
9 changes: 9 additions & 0 deletions .changeset/cold-bags-deny.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
'@sphinx-labs/contracts': patch
'@sphinx-labs/executor': patch
'@sphinx-labs/plugins': patch
'@sphinx-labs/core': patch
'@sphinx-labs/demo': patch
---

Add support for multisigs in the Sphinx config
4 changes: 0 additions & 4 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,6 @@ Here is the deployment process with the DevOps platform:

You can also use Sphinx's Hardhat or Foundry plugin as a feature-limited deployment tool. With this CLI tool, you can define your deployments in a declarative config file and generate deployment artifacts in the same format as `hardhat-deploy`. Your contracts will be deployed using `CREATE3`. However, you won't be able to use any of the other features described above.

### Coming soon

- Support for multisigs

## Documentation

### Getting Started
Expand Down
4 changes: 2 additions & 2 deletions docs/config-file.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ A config file looks like this:
orgId: '<org-id>',
testnets: ['goerli', 'optimism-goerli'],
mainnets: ['ethereum', 'optimism'],
owners: ['0x11111...'],
owners: ['0x11111...', '0x22222...', '0x33333...', ...],
ownerThreshold: 3,
proposers: ['0x9999...'],
},
Expand Down Expand Up @@ -103,7 +103,7 @@ It contains the following fields:
* Polygon Mumbai: `'maticmum'`
* BNB Smart Chain Testnet: `'bnbt'`
* Gnosis Chiado: `'gnosis-chiado'`
* `owners`: The list of addresses that own this project. Owners can perform permissioned actions such as approving deployments via the Sphinx UI. We currently only support projects that are owned by a single account.
* `owners`: The list of addresses that own this project. Owners can perform permissioned actions such as approving deployments via the Sphinx UI. We recommend that the owner accounts are hardware wallets.
* `ownerThreshold`: The number of owners that must sign a permissioned action, such as approving a deployment, before the action can be executed on-chain.
* `proposers`: The list of addresses that are allowed to propose changes to the Sphinx config file. Any change to the Sphinx config file, including contract deployments, must be proposed before it can be approved by the project owners. We recommend that proposals occur in a CI process, but you can also propose from the command line.

Expand Down
7 changes: 4 additions & 3 deletions packages/contracts/contracts/SphinxAuth.sol
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ contract SphinxAuth is AccessControlEnumerableUpgradeable, Semver {
error NotEnoughSignatures();
error InvalidSignatureLength();
error UnauthorizedSigner();
error DuplicateSigner();
error NonAscendingSignerOrder();
error InvalidToAddress();
error InvalidChainId();
error InvalidLeafIndex();
Expand Down Expand Up @@ -129,7 +129,8 @@ contract SphinxAuth is AccessControlEnumerableUpgradeable, Semver {
* @param _manager Address of the SphinxManager contract.
* @param _data Arbitrary data. Provides a flexible interface for future versions of this
contract. In this version, the data is expected to be the ABI-encoded
list of owners and owner threshold.
list of owners and the owner threshold. Note that the list of owners
should be in ascending order.
*/
function initialize(
address _manager,
Expand Down Expand Up @@ -705,7 +706,7 @@ contract SphinxAuth is AccessControlEnumerableUpgradeable, Semver {

signer = ECDSAUpgradeable.recover(typedDataHash, signature);
if (!hasRole(_verifyingRole, signer)) revert UnauthorizedSigner();
if (signer <= prevSigner) revert DuplicateSigner();
if (signer <= prevSigner) revert NonAscendingSignerOrder();

prevSigner = signer;
}
Expand Down
5 changes: 5 additions & 0 deletions packages/contracts/contracts/SphinxAuthFactory.sol
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,11 @@ contract SphinxAuthFactory is Ownable {
_transferOwnership(_owner);
}

/**
* @notice Deploys a new SphinxAuthProxy and initializes it with the given `_authData`.
*
* @param _authData Encoded data used to initialize the SphinxAuth contract.
*/
function deploy(
bytes memory _authData,
bytes memory _registryData,
Expand Down
5 changes: 5 additions & 0 deletions packages/core/src/addresses.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,6 +245,11 @@ export const getAuthData = (
owners: Array<string>,
ownerThreshold: number
): string => {
// Sort the owners in ascending order. This makes it easier to calculate the
// the address of the SphinxAuth contract, which is generated using the
// auth data.
owners.sort()

return utils.defaultAbiCoder.encode(
['address[]', 'uint256'],
[owners, ownerThreshold]
Expand Down
58 changes: 10 additions & 48 deletions packages/core/src/config/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,9 +44,9 @@ import {
fetchAndCacheCompilerConfig,
getConfigArtifactsRemote,
getDuplicateElements,
hyperlink,
getNetworkType,
resolveNetwork,
sortHexStrings,
} from '../utils'
import {
ParsedConfigVariable,
Expand Down Expand Up @@ -2935,47 +2935,6 @@ export const assertValidConfigOptions = (
)
}

// These are temporary until we add support for multisigs.
if (owners.length > 1) {
logValidationError(
'error',
`We currently only support configs that contain a single owner. Join our ${hyperlink(
'Discord',
'https://discord.gg/7Gc3DK33Np'
)} to \n` + `request this feature.`,
[],
cre.silent,
cre.stream
)
}
if (proposers.length > 1) {
logValidationError(
'error',
`We currently only support configs that contain a single proposer. Join our ${hyperlink(
'Discord',
'https://discord.gg/7Gc3DK33Np'
)} to \n` + `request this feature.`,
[],
cre.silent,
cre.stream
)
}
if (
ethers.utils.getAddress(owners[0]) !== ethers.utils.getAddress(proposers[0])
) {
logValidationError(
'error',
`We currently only support configs that have the same proposer and owner address. Join\n` +
`our ${hyperlink(
'Discord',
'https://discord.gg/7Gc3DK33Np'
)} to request this feature.`,
[],
cre.silent,
cre.stream
)
}

assertNoValidationErrors(failureAction)
}

Expand All @@ -2990,12 +2949,15 @@ export const parseConfigOptions = (
: mainnets.map((network) => SUPPORTED_MAINNETS[network])

// Converts addresses to checksummed addresses and sorts them in ascending order.
const owners = options.owners
.map((address) => ethers.utils.getAddress(address))
.sort()
const proposers = options.proposers
.map((address) => ethers.utils.getAddress(address))
.sort()
const owners = options.owners.map((address) =>
ethers.utils.getAddress(address)
)
sortHexStrings(owners)

const proposers = options.proposers.map((address) =>
ethers.utils.getAddress(address)
)
sortHexStrings(proposers)

return {
chainIds,
Expand Down
21 changes: 10 additions & 11 deletions packages/core/src/tasks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -223,20 +223,20 @@ export const proposeAbstractTask = async (
)
}

if (leafs.length === 0) {
spinner.succeed(
`Skipping proposal because your Sphinx config file has not changed.`
)
return
}

if (!cre.confirm && !dryRun) {
spinner.stop()
// Confirm deployment with the user before proceeding.
await userConfirmation(getDiffString(diff))
spinner.start(`Proposal in progress...`)
}

if (leafs.length === 0) {
spinner.succeed(
`Proposal completed, nothing has changed so no new deployment was created.`
)
return
}

const chainIdToNumLeafs: { [chainId: number]: number } = {}
for (const leaf of leafs) {
const { chainId } = leaf
Expand Down Expand Up @@ -389,13 +389,12 @@ export const proposeAbstractTask = async (

if (!dryRun) {
const websiteLink = blue(hyperlink('website', WEBSITE_URL))
spinner.succeed(
`Proposal succeeded! Go to the ${websiteLink} to approve the deployment.`
)

await relayProposal(proposalRequest)
const compilerConfigArray = Object.values(compilerConfigs)
await relayIPFSCommit(apiKey, orgId, compilerConfigArray)
spinner.succeed(
`Proposal succeeded! Go to the ${websiteLink} to approve the deployment.`
)
} else {
spinner.succeed(`Proposal dry run succeeded!`)
}
Expand Down
26 changes: 22 additions & 4 deletions packages/core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1255,19 +1255,19 @@ export const relayProposal = async (proposalRequest: ProposalRequest) => {
throw new Error(`Malformed Request: ${e.response.data}`)
} else if (e.response.status === 401) {
throw new Error(
`Unauthorized, please check your API key and Org Id are correct`
`Unauthorized request. Please check your Sphinx API key and organization ID are correct.`
)
} else if (e.response.status === 409) {
throw new Error(
`Unsupported network, please report this to the developers`
`Unsupported network. Please report this to the developers.`
)
} else if (e.response.status === 500) {
throw new Error(
`Internal server error, please report this to the developers`
`Internal server error. Please report this to the developers.`
)
} else {
throw new Error(
`Unexpected response code, please report this to the developers`
`Unexpected response code. Please report this to the developers.`
)
}
}
Expand Down Expand Up @@ -1585,3 +1585,21 @@ export const getNetworkNameForChainId = (chainId: number): string => {

return network
}

/**
* @notice Sorts an array of hex strings in ascending order. This function mutates the array.
*/
export const sortHexStrings = (arr: Array<string>): void => {
arr.sort((a, b) => {
const aBN = ethers.BigNumber.from(a)
const bBN = ethers.BigNumber.from(b)

if (aBN.lt(bBN)) {
return -1
} else if (aBN.gt(bBN)) {
return 1
} else {
return 0
}
})
}
131 changes: 39 additions & 92 deletions packages/plugins/contracts/foundry/SphinxConstants.sol

Large diffs are not rendered by default.

Loading

0 comments on commit eb0cc1c

Please sign in to comment.