Skip to content

add safe singleton factory to genesis#399

Closed
dvush wants to merge 1 commit intomainfrom
singleton-factory
Closed

add safe singleton factory to genesis#399
dvush wants to merge 1 commit intomainfrom
singleton-factory

Conversation

@dvush
Copy link
Contributor

@dvush dvush commented Mar 9, 2026

This is very useful because you can deploy all contact on the same address on all chains with this https://github.com/safe-fndn/safe-singleton-factory

@dvush dvush requested a review from canercidam as a code owner March 9, 2026 08:23
@claude
Copy link
Contributor

claude bot commented Mar 9, 2026

Claude finished @dvush's task in 53s —— View job


PR Review

  • Gather context and read changed files
  • Analyze changes and provide review

Summary

The PR adds the Safe Singleton Factory (a deterministic CREATE2 deployer) to both L1 and L2 genesis allocations. The implementation is clean and straightforward — defines the canonical address and runtime bytecode as package-level vars and injects them in both genesis paths.

Issues

  1. L1 ordering risk — On L1, the factory is injected before Optimism allocs are applied via maps.Copy (line 277). If OP allocs ever contain this address, the factory gets silently overwritten. Left an inline comment suggesting to move it after the OP allocs merge.

  2. components.go change is noise — The only change in components.go is reordering the _ "embed" import (moving a blank line). This is cosmetic and unrelated to the feature. Consider dropping it from this PR to keep the diff focused.


}

// add Safe Singleton Factory (deterministic CREATE2 deployer)
appendSafeSingletonFactoryToAlloc(&gen.Alloc)
Copy link
Contributor

Choose a reason for hiding this comment

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

The Safe Singleton Factory is added before Optimism L1 allocs are applied via maps.Copy (line 277). If the Optimism allocs ever include the same address (0x914d7Fec6aaC8cd542e72Bca78B30650d45643d7), maps.Copy will silently overwrite it. Consider moving this call after the Optimism allocs are applied, or at least after line 277, to ensure the factory is always present.

Copy link
Member

Choose a reason for hiding this comment

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

@dvush WDYT?

@dvush
Copy link
Contributor Author

dvush commented Mar 9, 2026

Its better for users to add is via predeploy feature of the playground

@dvush dvush closed this Mar 9, 2026
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