search

paritytech / substrate

Substrate: The platform for blockchain innovators
Apache License 2.0
8.39k stars 2.65k forks source link

Unify trim_compact_length and trim_compact_weight functions #8362

Closed coriolinus closed 2 years ago

coriolinus commented 3 years ago

Given that this code runs offchain, not priority but I can clearly see benefit in merging this and compact_weight_trim, given that I think we need to re-do this sorting in both of them, among other duplicate work.

Maybe you can formulate this as a follow issue for yourself.

_Originally posted by @kianenigma in https://github.com/paritytech/substrate/pull/8317#discussion_r594227532_

There's a lot of duplicate work in those two functions; we can simplify by unifying them.

Could also be worth combining the storage structs as well to reduce read costs, while we're at it.

stale[bot] commented 3 years ago

Hey, is anyone still working on this? Due to the inactivity this issue has been automatically marked as stale. It will be closed if no further activity occurs. Thank you for your contributions.

kianenigma commented 3 years ago

I'm still keen on this. It will simplify the code a lot.

coriolinus commented 3 years ago

Honestly, the ideal resolution will be to get rid of both of them entirely, and rely in pre-trimming via max_allowed_voters to ensure the solution fits. Post-trimming as we do here is both kind of expensive, and a bit risky.

On Thu, Jul 8, 2021 at 11:07 AM Kian Paimani @.***> wrote:

I'm still keen on this. It will simplify the code a lot.

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/paritytech/substrate/issues/8362#issuecomment-876269531, or unsubscribe https://github.com/notifications/unsubscribe-auth/AB3V4TUZR22DID3UBOBVUZTTWVTGLANCNFSM4ZGSB7XA .

-- Wichtige Mitteilung

Diese Mitteilung wurde von Parity Technologies Deutschland GmbH kommuniziert, eine im Handelsregister des Amtsgerichtes Charlottenburg unter HRB 190583 B registrierte Gesellschaft mit beschränkter Haftung (GmbH). Der Geschäftsführer der GmbH ist Herr Raul Romanutti. Der registrierte Geschäftssitz ist Glogauer Straße 6 in 10999 Berlin, Deutschland.

Diese Mitteilung enthält Informationen welche vertraulich sind und welche eventuell die Vertraulichkeit der Rechtsberatung ("Anwaltsgeheimnis") berühren. Sie ist ausschließlich für den/die vorgesehenen Empfänger bestimmt. Wenn Sie nicht der/die beabsichtigte(n) Empfänger sind, benachrichtigen Sie bitte @. @.> und löschen Sie diese Nachricht sofort.

Unsere Datenschutzrichtlinie, einschließlich die Art und den Umfang von personenbezogenen Daten, die wir erfassen, wie wir diese Daten erfassen und verarbeiten, an wen wir sie in Bezug auf die von uns angebotenen Dienste weitergeben dürfen, sowie bestimmte Rechte und Optionen, die Sie in dieser Hinsicht haben, finden Sie unter: https://www.parity.io/privacy/ https://www.parity.io/privacy/

kianenigma commented 2 years ago

We might ditch this, since we will soon get a new election pallet and we don't want to support multi-phase forever.