-
Notifications
You must be signed in to change notification settings - Fork 0
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
[WIP] Code Jar Improvements, Deploy with Code Jar, and Verification #127
Conversation
This patch improves CodeJar to deploy a new contract `CodeJarStub` instead of building the EVM opcodes by hand. This is because we can use `return()` yul-opcode in constructors to build custom code (weird, right!?). From this, we also build a way of deploying contracts using CodeJar (that is: content-addressible contracts-- the deploy address is based on the actual opcodes of the contract you are deploying). Further, we work on `verify` functionality for these contracts, which works if (and right now, only if) the deployed contract doesn't use `const immutable`s. There are reasons this could break otherwise, but for simple contracts, it works.
@@ -0,0 +1,8 @@ | |||
// SPDX-License-Identifier: BSD-3-Clause |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This whole contract can be removed before we merge.
return address( | ||
uint160(uint256(keccak256(abi.encodePacked(bytes1(0xff), address(this), uint256(0), keccak256(initCode))))) | ||
uint160(uint256(keccak256(abi.encodePacked(bytes1(0xff), address(this), uint256(0), keccak256(abi.encodePacked(type(CodeJarStub).creationCode, abi.encode(code))))))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note the abi.encode()
here is encoding as it's actual type bytes
and thus prepending length, etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be 0x0000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000{len}{...bytes}
I believe
contract CodeJarStub { | ||
constructor(bytes memory code) { | ||
assembly { | ||
return(add(code, 0x20), mload(code)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Funny that this works. It doesn't work as well with const immutables in the contract, though.
|
||
vm.startBroadcast(deployer); | ||
|
||
console.log("============================================================="); | ||
console.log("Deploying QuarkWalletFactory"); | ||
|
||
quarkWalletFactory = new QuarkWalletFactory(); | ||
quarkWalletFactory = QuarkWalletFactory(deploy(type(QuarkWalletFactory).creationCode, hex"", deployerPk)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just a test to see if we could deploy this way. We probably don't want to...
contract_src="./src/$contract.sol" | ||
contract_deployed_src="./src/${contract_deployed}.sol" | ||
|
||
cat "$contract_src" \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Verify is a little weird for a fun reason. It makes a copy of your contract (e.g. SimpleDeployed.sol
), changes the constructor to match CodeJarStub
, compiles that new contract, and verifies that contract at the address. It's pretty nifty that it kinda works. Note: we don't use solc
directly here to compile since we want to mirror your foundry settings.
constructor_args=`cast abi-encode "cons(bytes)" $deployed_code` | ||
|
||
contract_src="./src/$contract.sol" | ||
contract_deployed_src="./src/${contract_deployed}.sol" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is obviously a bit too specific to using ./src
etc, though this stuff is a little hacky all around anyway
@@ -95,6 +95,8 @@ contract CodeJarTest is Test { | |||
hex"00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff"; | |||
scripts[7] = | |||
hex"00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff00112233445566778899aabbccddeeff11"; | |||
scripts[8] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
wanted something really big to see that it didn't change nothing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
honestly, I found the previous version more intuitive. I guess solidity understands this version well enough that we can, more-or-less, have verification?
// ETHERSCAN_KEY | ||
|
||
contract CodeJarDeployer is Script { | ||
address constant DEFAULT_CODE_JAR = address(0xcA85333B65E86d114e2bd5b4aE23Fe6E6a3Ae8e3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how exactly is this determined? is this a goerli codejar address? 🤔
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it. But the plan is to have this be the same address on each chain
elif [[ ${constructor_abi} =~ ^\([[:alnum:]_,]+\)$ ]]; then | ||
constructor_abi="cons$constructor_abi" | ||
elif [[ ${constructor_abi} =~ ^[[:alnum:]_,]+$ ]]; then | ||
constructor_abi="cons($constructor_abi)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this how the solidity constructor is invoked, using a selector for the string cons
? I really cannot tell if this is an implied default, which assumes that you've implemented a method named cons
, or if it will invoke a constructor() {..}
as defined in solidity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
weirdly cast abi-encode
both requires and ignores it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need to clearly document the "when" and "why" of codejar deploys, because I am still unclear on whether the primary benefit of deterministic addresses (that is, dependent upon the address of codejar itself) is worth it.
For Quark scripts, I think "yes," since the refactors allow us to verify codejar-deployed scripts. That's a clear win.
For everything else, although we keep talking about how it would be cool to deploy with codejar, the answer feels like "no" to me.
If we could make the entire constellation of Quark contracts content-addressible, it would feel like more of a win: codejar would be the deployer, the manifest for finding other contracts, the entrypoint to everything. However, if even one contract is not content-addressible, it's actually harder to find things. Today, all you need to know is the factory address, and everything else can be derived from there. If half of everything is codejar-derived, and the rest is factory-derived, it's just annoying.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting approach 🙌
Have you tried running the deploy script with constructor args?
assembly { | ||
codeCreateAddress := create2(0, add(initCode, 32), initCodeLen, 0) | ||
} | ||
CodeJarStub codeCreateAddress = new CodeJarStub{salt: 0}(code); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Can remove the address
suffix since this isn't an address. One suggestion is to name this createdContract
.
* @dev Returns the create2 address based on the given initCode | ||
* @return The create2 address based on running the initCode constructor |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Comments need to be updated
); | ||
} | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: extra line
// ETHERSCAN_KEY | ||
|
||
contract CodeJarDeployer is Script { | ||
address constant DEFAULT_CODE_JAR = address(0xcA85333B65E86d114e2bd5b4aE23Fe6E6a3Ae8e3); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it. But the plan is to have this be the same address on each chain
} | ||
|
||
// TODO: Consider metadata and verification | ||
bytes memory deployedCodeWithMetadata = abi.encodePacked(deployedCode, hex""); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hex""
is reserved for the metadata? Are there restrictions on what it can be?
codeJar = CodeJar(codeJarAddr); | ||
} | ||
|
||
address initCodeAddr = codeJar.saveCode(abi.encodePacked(contractCode, constructorArgs)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very cool!
This patch adds `det`, which is a proxy that always signs with the same exact signature. This allows for deterministic deployment addresses since it's a variety of content-addressable signatures. See the README.md in `det/` for more details. Note: I can't get it to work right now, but it... should... work? Like it's accepted to the mempool _then ignored_ which might indicate that the signature is not, in fact, valid, but looks valid to EthersJS and Infura.
This patch improves CodeJar to deploy a new contract
CodeJarStub
instead of building the EVM opcodes by hand. This is because we can usereturn()
yul-opcode in constructors to build custom code (weird, right!?). From this, we also build a way of deploying contracts using CodeJar (that is: content-addressible contracts-- the deploy address is based on the actual opcodes of the contract you are deploying). Further, we work onverify
functionality for these contracts, which works if (and right now, only if) the deployed contract doesn't useconst immutable
s. There are reasons this could break otherwise, but for simple contracts, it works.