Skip to content
This repository has been archived by the owner on Nov 8, 2023. It is now read-only.

Add counterfactual deployment test case #115

Merged
merged 4 commits into from
Oct 18, 2023
Merged

Add counterfactual deployment test case #115

merged 4 commits into from
Oct 18, 2023

Conversation

lalexgap
Copy link
Contributor

@lalexgap lalexgap commented Oct 18, 2023

⚠️ Based on #110
This adds a test case to test the Entrypoint contract deploying our SCBridgeWallet on demand using our contract factory. Instead of deploying the SCBridgeWallet before submitting the UserOperation, the test specifies the initcode which points to our factory. When the EntryPoint receives the UserOperation it checks the precomputed address and deploys the SCBridgeWallet for us.

TODO:

  • Update our clients to use this initCode and update our deploy script to just prefund the SCBridgeWallet addresses instead of deploying to them.

@lalexgap lalexgap marked this pull request as draft October 18, 2023 05:02
@lalexgap

This comment was marked as outdated.

@lalexgap
Copy link
Contributor Author

Hmm, I thought I had this working but it looks like the test is still failing.

Ah, it looks like I accidentally deleted a line when adding a helper, it has now been resolved.

@lalexgap lalexgap marked this pull request as ready for review October 18, 2023 05:08
const userOp: UserOperationStruct = {
sender: precomputedSCWAddress,
nonce: 0,
initCode,
Copy link
Contributor

Choose a reason for hiding this comment

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

Checking my understanding: won't it be a waste of gas to include this initcode with every l1 userop ?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think so, yes. You want to include it only if the wallet isn't already deployed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point! We'd pay slightly more gas due to the increased callData size of including the initCode if we included it with every UserOperation.

Comment on lines +24 to +44
address(
uint160(
uint(
keccak256(
abi.encodePacked(
bytes1(0xff),
address(this),
salt,
keccak256(
abi.encodePacked(
type(SCBridgeWallet).creationCode,
abi.encode(owner),
abi.encode(intermediary),
abi.encode(entrypoint)
)
)
)
)
)
)
);
Copy link
Contributor

Choose a reason for hiding this comment

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

A beautiful pyramid calculation.

@NiloCK
Copy link
Contributor

NiloCK commented Oct 18, 2023

Really great - thanks @lalexgap.

Co-authored-by: Colin Kennedy <[email protected]>
@lalexgap lalexgap merged commit a907501 into saner-defaults Oct 18, 2023
1 check passed
@lalexgap lalexgap deleted the no-deploy branch October 18, 2023 16:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants