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.

Sign up for GitHub

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

Jump to bottom

refactor: ♻️ storage-proofs -> proofs-dealer-trie #12

Merged
merged 8 commits into from
Mar 19, 2024
Merged

refactor: ♻️ storage-proofs -> proofs-dealer-trie #12

merged 8 commits into from
Mar 19, 2024

Conversation

ffarall
Copy link
Contributor

@ffarall ffarall commented Mar 19, 2024

This PR:

  • Makes the old storage-proofs pallet be abstracted over the fact that the providers are storage providers (now they're just "Providers").
  • Defines a trait to be implemented by another pallet, that would give it information of the Providers. This trait should be implemented by the Storage Providers pallet.

snowmead
snowmead approved these changes Mar 19, 2024
View reviewed changes
Copy link
Contributor

@snowmead snowmead left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I like the new name 👍🏼

I think pallet-proofs-dealer would be better, I don’t mind if you leave it as is.

pallets/storage-proofs/src/lib.rs Outdated Show resolved Hide resolved
pallets/storage-proofs/src/lib.rs Outdated Show resolved Hide resolved
TDemeco
TDemeco requested changes Mar 19, 2024
View reviewed changes
Copy link
Contributor

@TDemeco TDemeco left a comment

Choose a reason for hiding this comment

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

LGTM but I left some comments that are important for a trait implementation in pallet-providers

pallets/storage-proofs/src/lib.rs Outdated Show resolved Hide resolved
pallets/storage-proofs/src/lib.rs Show resolved Hide resolved
@TDemeco TDemeco self-requested a review March 19, 2024 17:38
TDemeco
TDemeco approved these changes Mar 19, 2024
View reviewed changes
Copy link
Contributor

@TDemeco TDemeco left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@ffarall ffarall merged commit 95606e6 into main Mar 19, 2024
4 checks passed
@ffarall ffarall deleted the refactor/proofs-dealer-trie branch March 19, 2024 17:49
@ffarall ffarall mentioned this pull request Mar 19, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@snowmead snowmead snowmead approved these changes

@TDemeco TDemeco TDemeco approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@ffarall @TDemeco @snowmead