Skip to content
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

Factory uses bytes4 to track instanceTypes, this is unjustifiably restrictive #278

Open
fulldecent opened this issue Nov 18, 2019 · 1 comment
Labels
Breaking Changes Breaking changes to the interface or architecture
Milestone

Comments

@fulldecent
Copy link
Contributor

The Factory contract wants to track types of contracts. It does this using a bytes4 type. It seems this is inspired from the ERC-165 standard.

The baseline choice should be using a full 32 byte hash. The bytes4 costs more gas to store, retrieve and compare (EVM stores a whole word, Solidity handles truncation) also it has less collision resistance.

As an author of ERC-165 I think bytes4 is a bad choice here.

Recommendation: use a full word hash

References: Definition

bytes4 private _instanceType;

@thegostep thegostep added this to the v2.0.0 milestone Nov 20, 2019
@thegostep
Copy link
Contributor

Change would require redeployment of registries, added for v2

@thegostep thegostep added the Breaking Changes Breaking changes to the interface or architecture label Nov 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Changes Breaking changes to the interface or architecture
Projects
None yet
Development

No branches or pull requests

2 participants