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

Do not allow any crowdloan contributions during the VRF period #3346

Merged
merged 11 commits into from
Jun 24, 2021

Conversation

shawntabrizi
Copy link
Member

@shawntabrizi shawntabrizi commented Jun 22, 2021

After the ending period of an auction, there is a delay needed to select the random block used to win the auction.

During this time, a crowdloan could receive contributions which would not help win the auction whatsoever.

Because we cannot identify which crowdloans would win a current auction, we must stop all crowdloans from accepting contributions during this VRF delay period.

This PR refactors the way that the Auction pallet and the Auctioneer trait reports the status of an auction, making it easy to detect these scenarios, and improving the overall code structure and readability.

This PR also repairs a small bug where the EndingsCount was being incremented multiple times per auction. No harm done here, but this is a proper fix for that behavior.

@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Jun 22, 2021
@shawntabrizi shawntabrizi added B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. labels Jun 22, 2021
@emostov
Copy link
Contributor

emostov commented Jun 23, 2021

LGTM - the changes are a nice QoL improvement

@kianenigma kianenigma self-requested a review June 24, 2021 17:37
@@ -855,6 +863,12 @@ mod tests {
fn bids() -> Vec<BidPlaced> {
BIDS_PLACED.with(|p| p.borrow().clone())
}
fn vrf_delay() -> u64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

unfortunately no one ever uses them 😁 but I added some pretty convenient ways to create all of these thread local stuff with parameter_types! with getter and setters https://crates.parity.io/frame_support/macro.parameter_types.html

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually i do normally use it, but copy paste from local code is faster than looking up the syntax lol

Copy link
Contributor

@kianenigma kianenigma left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@shawntabrizi shawntabrizi added D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. and removed D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited. labels Jun 24, 2021
Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@shawntabrizi
Copy link
Member Author

bot merge

@ghost
Copy link

ghost commented Jun 24, 2021

Trying merge.

@shawntabrizi shawntabrizi merged commit a6d646b into master Jun 24, 2021
@shawntabrizi shawntabrizi deleted the shawntabrizi-auction-patch branch June 24, 2021 21:24
shawntabrizi added a commit that referenced this pull request Jun 25, 2021
* patch `is_ending` logic for sampling

* Create AuctionStatus abstraction

* clean up apis

* fix docs

* Add check for contributions during vrf

* custom error for this

* fix benchmark

* opening -> starting

* Apply suggestions from code review

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>

* Update runtime/common/src/auctions.rs

* avoid divide by zero stuff

Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com>
@mmostafas mmostafas added D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited. and removed D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. labels Jun 28, 2021
ordian added a commit that referenced this pull request Jul 2, 2021
* master: (21 commits)
  cleanup stream polls (#3397)
  Staking Miner (#3141)
  Companion for Substrate#8953 (#3140)
  Bump version, specs & substrate in prep for v0.9.8 (#3387)
  Fix busy loops. (#3392)
  Minor refactor (#3386)
  add simnet tests (#3381)
  BEEFY: adjust gossip (#3372)
  Companion for #9193 (#3376)
  Companion for Decouple Staking and Election - Part 3: Signed Phase (#2793)
  Ensure that we fetch another collation if the first collation was invalid (#3362)
  Only send one collation per relay parent at a time to validators (#3360)
  disable approval-checking-grandpa on dev chain (#3364)
  Use associated constant for max (#3375)
  Use wasm-builder from git (#3354)
  Squashed 'bridges/' changes from b2099c5..23dda62 (#3369)
  Bump versions & spec_versions (#3368)
  Don't allow bids for a ParaId where there is an overlapping lease period (#3361)
  Companion for upgrade of transaction-payment to pallet macro (#3267)
  Do not allow any crowdloan contributions during the VRF period (#3346)
  ...
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D1-audited 👍 PR contains changes to critical logic that has been properly reviewed and externally audited.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants