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

Remmediate [PIH-01M] Inexistent Support of Manual Seller Configuration #747

Merged
merged 11 commits into from
Aug 25, 2023

Conversation

zajck
Copy link
Member

@zajck zajck commented Aug 3, 2023

Closes #728

Based on #746 which should be reviewed and merged first.

Instead of backfilling the data at the initialization time, seller salts are now generated

  • when a seller is created for new sellers
  • dynamically the first time the new collection is created for existing sellers.
    • Existing sellers can also manually set it by calling updateSellerSalt

Due to the changes in #746 it will be possible that the same wallet creates multiple seller ids. It's also possible that already now there exist seller accounts with the same creator address. To avoid collisions, seller salt is now derived from the admin address and the user-supplied salt.
Since this is a user-defined value, we cannot just pick it on the sellers' behalf.

Two view methods were added to check if the chosen salt will result in successful clone creation:

  • isSellerSaltAvailable can be used before calling createSeller, createNewCollection (for sellers from before v2.3.0 which might not have seller salt initialized yet) or updateSellerSalt
  • calculateCollectionAddress can be used to get both the expected collection address and its availability.

@zajck zajck self-assigned this Aug 3, 2023
@zajck zajck added the v2.3.0 label Aug 3, 2023
@coveralls
Copy link

coveralls commented Aug 3, 2023

Coverage Status

coverage: 99.571% (+0.005%) from 99.566% when pulling 7c277e7 on audit_v2_3_0_pih_01_m into 58dddcb on main.

Copy link
Member

@levalleux-ludo levalleux-ludo left a comment

Choose a reason for hiding this comment

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

@zajck I've left a small question

contracts/protocol/facets/SellerHandlerFacet.sol Outdated Show resolved Hide resolved
Copy link
Contributor

@albertfolch-redeemeum albertfolch-redeemeum left a comment

Choose a reason for hiding this comment

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

LGTM

contracts/protocol/facets/SellerHandlerFacet.sol Outdated Show resolved Hide resolved
contracts/protocol/facets/SellerHandlerFacet.sol Outdated Show resolved Hide resolved
contracts/protocol/facets/SellerHandlerFacet.sol Outdated Show resolved Hide resolved
@levalleux-ludo levalleux-ludo dismissed stale reviews from albertfolch-redeemeum and themself via 7c277e7 August 25, 2023 08:51
Co-authored-by: albertfolch-redeemeum <102516373+albertfolch-redeemeum@users.noreply.github.com>
@levalleux-ludo levalleux-ludo merged commit 64e08ac into main Aug 25, 2023
9 checks passed
@zajck zajck deleted the audit_v2_3_0_pih_01_m branch August 25, 2023 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[PIH-01M] Inexistent Support of Manual Seller Configuration
4 participants