Using `safe-singleton-factory` for module deployment

safe-global / safe-modules

A collection of modules that can be used with the Safe contract

GNU Lesser General Public License v3.0

151 stars 74 forks source link

Using `safe-singleton-factory` for module deployment #477

Closed remedcu closed 3 months ago

remedcu commented 3 months ago

This PR adds the use of safe-singleton-factory for deploying the modules.

Note:

allowances already used the safe-singleton-factory for deployment.
passkey version was not updated, as it was not deployed to any network AFAIK.
Lock file was updated due to the addition of safe-singleton-factory in the root level. (Should we just install it individually for all 4 modules?)

Open Question: Should we update the version of the earlier deployed contracts or just overwrite it in CHANGELOG? Personally think that CHANGELOG should contain the change.

Follow-Up Question: If we are updating the version, then should we make a new release of the npm package?

Fixes #473

coveralls commented 3 months ago

Pull Request Test Coverage Report for Build 10353559880

Details

0 of 0 changed or added relevant lines in 0 files are covered.
No unchanged relevant lines lost coverage.
Overall coverage remained the same at 100.0%

Totals
Change from base Build 10214202563:	0.0%
Covered Lines:	51
Relevant Lines:	51

💛 - Coveralls

akshay-ap commented 3 months ago

Should we just install it individually for all 4 modules?

Nope. I think keeping this package as root dependency is the right way we also expect other future modules to get deployed using this factory.

Personally think that CHANGELOG should contain the change.

Agreed

If we are updating the version, then should we make a new release of the npm package?

As there are no other changes, I think we can do version upgrade and release with other changes in future for each npm package.

remedcu commented 3 months ago

LGTM - can we just create a GH issue to track #477 (comment)?

https://github.com/safe-global/safe-modules/issues/481