-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Explicitly Handling ProvisionableData Cases #6757
Conversation
|
node/core/provisioner/src/lib.rs
Outdated
// backing stage, can't be guaranteed to conclude. Also candidates which haven't | ||
// been made available don't pose a security risk as they can not be included, | ||
// approved, or finalized. Thus we wait to punish misbehavior until a candidate | ||
// reaches the approval checking stage. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The main point here is, we cannot dispute at this stage. What the backer could do is record the fact that the candidate was invalid and dispute it once it gets included. This would add to security a bit, but not too much as backers are known ahead of time and we assume they can be dosed. Anyhow if we decide the complexity is worth it, we can do it at some point: #3232 <- Referencing this ticket here might make sense as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great, thanks for clarifying things! I don't see a reason to change Cargo.lock
in this PR though, as it's an unrelated change. Can you undo that?
node/core/provisioner/src/lib.rs
Outdated
@@ -324,7 +324,23 @@ fn note_provisionable_data( | |||
.with_para_id(backed_candidate.descriptor().para_id); | |||
per_relay_parent.backed_candidates.push(backed_candidate) | |||
}, | |||
_ => {}, | |||
// We choose to do nothing with misbehavior at this stage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should argue why not here as well. The misbehavior is self-contained and the validator could get punished. The reason we don't is, because we already have mitigation on the protocol level (reputation changes) and for the time being those seem to be enough.
bot merge |
Error: Statuses failed for 290b63b |
bot merge |
Error: Statuses failed for 290b63b |
bot merge |
Error: Statuses failed for 290b63b |
* master: (98 commits) Ensure max_weight is assigned properly in AllowTopPaidExecutionFrom (#6787) Explicitly Handling ProvisionableData Cases (#6757) Companion for Substrate#12520 (#6730) Revert back to bare metal runners for weights generation (#6762) Improve XCM fuzzer (#6190) Corrected weight trader comment (#6752) clean up executed migrations (#6763) Remove state migration from westend runtime. (#6737) polkadot companion #12608 (Pools claim permissions) (#6753) Add Turboflakes bootnodes to Polkadot, Kusama and Westend (#6628) Companion for substrate#13284 (#6653) Companion for Substrate #13410: Introduce EnsureOrigin to democracy.propose (#6750) `BlockId` removal: `BlockBuilderProvider::new_block_at` (#6734) Companion PR for PR#13119 (#6683) Companion for Substrate#13411: frame/beefy: prune entries in set id session mapping (#6743) `BlockId` removal: refactor of runtime API (#6721) Fix auction bench (#6747) Use PVF code paired with executor params wherever possible (#6742) Retire `OldV1SessionInfo` (#6744) Companion for substrate #13121 - BEEFY Equivocations support (#6593) ...
ProvisionerMessage::ProvisionableData
can pass along two data variants for which we want to do nothing. Previously these were ignored using a wildcard, which is bad form.This PR explicitely handles those cases, explaining why we don't need to address them during the backing phase. A couple lines of logging and implementer's guide changes accompany.