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

Preparation: Do not announce an approval assignment if the code is not prepared #910

Open
pepyakin opened this issue Nov 15, 2021 · 6 comments

Comments

@pepyakin
Copy link
Contributor

If a validator restarted compiled code cache is wiped. Thus before a candidate-validation request the PVF should be prepared anew.

In the light of recent changes to the validation host (paritytech/polkadot#4273 and paritytech/polkadot#4270), the worst case time of candidate validation is increased.

Because of that we should avoid publishing our approval announcements until we get the preparation code ready.

@eskimor
Copy link
Member

eskimor commented Mar 3, 2023

@burdges Is this secure? Let's assume we have a parathread. Now we have a node upgrade and all nodes restart over the course of several days - let's assume that parathread never authored a block in that time. Now, after all validators upgraded, this parathread authors a block: No honest validators will now cast an assignment, because the PVF isn't prepared - all the malicious validators could of course send out their assignment immediately.

@sandreim you are more familiar with approval voting as well, what do you think?

@eskimor eskimor changed the title Do not announce an approval assignment if the code is not prepared Preparation: Do not announce an approval assignment if the code is not prepared Mar 3, 2023
@eskimor
Copy link
Member

eskimor commented Mar 3, 2023

We had a discussion about this issue recently - if one has a link, please post it here.

@sandreim
Copy link
Contributor

sandreim commented Mar 3, 2023

I am not 100% about how secure this would be, but we wouldn't see any no-shows. As you describe malicious validators wait for the right moment to and approve an invalid block. Also, the backing group could be fully malicious, or even just 2 malicous nodes can back an invalid candidate while the honest ones still compile the code. Given that validators in the backing group cannot vote in approval checking, then it is easy to see how invalid blocks can be included and finalized if we don't send out the assignments.

Without any malicious nodes, we will be slow and parachain block times would see a hit for first block(s) if compilation takes a lot of time during backing.

To mitigate this issue in most cases I would just not wipe the cache on restart and have it be a MRU of a reasonable size.

Additionally, a fallback I am thinking about is to actually start preparing the PVF as soon as we learn about the core being assigned to a para id for which we don't yet have it cached. We should have enough time to prepare the PVF, or at worst we would get no-shows.

@mrcnski
Copy link
Contributor

mrcnski commented Mar 3, 2023

Not sure if this is what you have in mind @eskimor, but there was a related discussion here about not clearing the cache on restart.

@eskimor
Copy link
Member

eskimor commented Mar 3, 2023

Not sure if this is what you have in mind @eskimor, but there was a related discussion here about not clearing the cache on restart.

Thank you! Exactly what I was looking for :-)

@burdges
Copy link

burdges commented Mar 3, 2023

I'd consider this insecure if PVFs can be updated too quickly. In principle, we could've "I'm still compiling" messages, but they'd complicate #742

We delay PVF updates by at least an epoch so nodes have time to compile them, right? Can we make honest nodes have all PVFs compiled when they stand for elections?

We've some pre-checking vote for PVF compilation, right? We've documented that somewhere?

We're kinda assuming that if PVFs pass pre-checking then all nodes eventually compile them correctly. We could've some "no-show me now because this crap does not build" message, but not sure this really solves anything.

@Sophia-Gold Sophia-Gold transferred this issue from paritytech/polkadot Aug 24, 2023
claravanstaden pushed a commit to Snowfork/polkadot-sdk that referenced this issue Dec 8, 2023
* upgrade tests

* updated weights

* Mock upgrade contract

* add smoketest for upgrade

* remove verbose print

* added smoketest

* updated error logging

* fixes

* better messages

* typos

* update readme

* moved file

* ws

* update paths

* update cumulus
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 26, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Mar 27, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 8, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 9, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
serban300 pushed a commit to serban300/polkadot-sdk that referenced this issue Apr 10, 2024
bkchr pushed a commit that referenced this issue Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Backlog
Development

No branches or pull requests

5 participants