Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Remove all bespoke implementations of preimage-lookup #12070

Closed
gavofyork opened this issue Aug 19, 2022 · 7 comments
Closed

Remove all bespoke implementations of preimage-lookup #12070

gavofyork opened this issue Aug 19, 2022 · 7 comments
Assignees
Labels
I7-refactor Code needs refactoring.

Comments

@gavofyork
Copy link
Member

gavofyork commented Aug 19, 2022

Avoid all unbounded preimage lookups since they are at risk from PoV-bloat-attack. Instead, use Preimages pallet, QueryPreimage/StorePreimage traits and Bounded<T>.

  • Multisig pallet's Calls (can have this functionality removed for simplicity).
  • Others?
@gavofyork gavofyork added the I7-refactor Code needs refactoring. label Aug 19, 2022
@gavofyork gavofyork mentioned this issue Aug 19, 2022
9 tasks
@gavofyork
Copy link
Member Author

@ggwpez I don't think there's any changes needed to be made here, but could you check through all production pallets and make sure we're not doing this anywhere?

@MrishoLukamba
Copy link
Contributor

I want to work on this

@ggwpez
Copy link
Member

ggwpez commented Oct 7, 2022

I think we have to tackle paritytech/polkadot-sdk#323, otherwise we have no guarantee that all storage items are bound.

@gavofyork
Copy link
Member Author

related: #12821

@gavofyork
Copy link
Member Author

@ggwpez would be good to close this - I suspect there are no further preimage lookups happening in the pallets, but would be good to confirm this and close.

@ggwpez
Copy link
Member

ggwpez commented Mar 16, 2023

Yes AFAIK we got all of them. Looking at the storage sizes of each pallet confirms this.
The PreimageProvider/PreimageRecipient traits could be deprecated now.

|             Pallet             |            Storage             | Max Values |    Max Size     |
| ------------------------------ | ------------------------------ | ---------- | --------------- |
| ImOnline                       | ReceivedHeartbeats             | None       | Some(10021032)  |
| Preimage                       | PreimageFor                    | None       | Some(4194344)   |
| Pov                            | LargeValue2                    | Some(1)    | Some(4194308)   |
| Pov                            | LargeValue                     | Some(1)    | Some(4194308)   |
| ImOnline                       | Keys                           | Some(1)    | Some(320002)    |
| Contracts                      | CodeStorage                    | None       | Some(126001)    |
| Contracts                      | PristineCode                   | None       | Some(125988)    |
| Scheduler                      | Agenda                         | None       | Some(107022)    |
| MessageQueue                   | Pages                          | None       | Some(65584)     |
| Nis                            | Queues                         | None       | Some(48022)     |
| TransactionStorage             | Transactions                   | None       | Some(36886)     |
| TransactionStorage             | BlockTransactions              | Some(1)    | Some(36866)     |
| ConvictionVoting               | VotingFor                      | None       | Some(27241)     |
| Alliance                       | UnscrupulousWebsites           | Some(1)    | Some(25702)     |
| NominationPools                | SubPoolsStorage                | None       | Some(24382)     |
| Democracy                      | PublicProps                    | Some(1)    | Some(16702)     |
| Contracts                      | DeletionQueue                  | Some(1)    | Some(16642)     |
| CoreFellowship                 | MemberEvidence                 | None       | Some(16429)     |
| Alliance                       | Announcements                  | Some(1)    | Some(8702)      |
| Babe                           | UnderConstruction              | None       | Some(8206)      |

(the Pov is a testing pallet)

@ggwpez ggwpez closed this as completed Mar 16, 2023
@ggwpez
Copy link
Member

ggwpez commented Mar 16, 2023

And the worst-case proof weights from master also look fine.
image

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
I7-refactor Code needs refactoring.
Projects
None yet
Development

No branches or pull requests

3 participants