Coretime RFC-17 implementation#11351
Coretime RFC-17 implementation#11351Szegoo wants to merge 19 commits intoparitytech:mertwole-rfc-17-pt1from
Conversation
|
One note about the PR: with the current code, the broker only supports the RFC‑17 sale model because some sale logic still isn’t fully abstracted out of the broker pallet. I’m working on refactoring this so the broker can work with both the previous sale implementation and RFC‑17. |
| for (id, bid) in Bids::<T>::iter() { | ||
| all_bids.push((id, bid)); | ||
| } | ||
| all_bids.sort_by(|a, b| b.1.price.cmp(&a.1.price)); |
There was a problem hiding this comment.
RFC-17 specifies random selection at the marginal step (clearing price) using parent block hash as entropy source.
This sort is stable, so equal-price bids maintain their order from the iterator. When N bids tie at the clearing price and only some can win, this is not random.
Something like Fisher-Yates shuffle?
| } | ||
|
|
||
| let has_renewal_rights = | ||
| T::RenewalRights::renewal_rights_count(&bid.who, sale.region_end) > 0; |
There was a problem hiding this comment.
renewal_rights_count() returns u32, but this collapses it to a bool without later decreasing the renewal rights count. If account A has 3 renewal rights and 4 winning bids, all 4 get has_renewal_rights = true and are protected from displacement.
Only the top N bids (by bid_price) should be protected, where N = renewal_rights_count. The 4th bid is displaceable
| /// The core index assigned to this allocation. | ||
| pub core: CoreIndex, | ||
| /// Whether this winner holds renewal rights and is protected from displacement. | ||
| pub has_renewal_rights: bool, |
There was a problem hiding this comment.
This should be renewal_rights_count: u32 otherwise we over-protects accounts with fewer renewal rights than winning bids
| // Opening price based on previous clearing price. | ||
| let previous_clearing = old_sale.clearing_price.unwrap_or(old_sale.reserve_price); | ||
| let opening_price = | ||
| previous_clearing.saturating_mul(T::PriceMultiplier::get().into()); |
There was a problem hiding this comment.
RFC-17 specifies opening_price = max(P_MIN, 3 * reserve_price), anchored to the current reserve price.
This uses previous_clearing × PriceMultiplier instead, which anchors to the previous clearing price. The dynamics are different — if previous sale was undersubscribed (clearing = reserve), this gives 2 * reserve vs RFC-17's 3 * reserve
| .iter() | ||
| .any(|a| matches!(a, TickAction::Refund { amount, who } if *who == 1 && *amount == excess))); | ||
|
|
||
| // Bidder 2 (mid_bid) loses and gets full refund. |
There was a problem hiding this comment.
bidder 2 (mid_bid) is a winner, not a loser with 2 cores offered
| .iter() | ||
| .filter(|a| matches!(a, TickAction::Refund { .. })) | ||
| .count(); | ||
| assert!(refund_count >= 1); |
There was a problem hiding this comment.
a bit loose. we can have a tighter assertion
3 participants
This PR is based on the refactoring in #10916.
Summary
The idea is to decouple the market implementation logic into a separate pallet. The broker pallet will remain responsible for region management (partitioning, interlacing, transfers, etc.), scheduling, and related functionality. The market pallet will be responsible for all sale-related logic.
We will proceed directly to implementing RFC-17, as the new versions of the broker and coretime market pallets will be available on AH, and we will deprecate the broker pallet on the Coretime chain. We don't really care about backward compatibility.
During the transition period to AH, instead of forking the broker pallet, both the Coretime chain and AH will use the same version of the broker but with different market implementations: the Coretime chain will keep the current sale implementation, while AH will use RFC‑17.
RFC-17 Implementation
Notable changes
Broker pallet:
raise_bidextrinsic so users can increase their bids during the auctiondo_renewduring Market phase fails with WrongPhase, which defers auto-renewals to the Renewal phaseCoretime market pallet:
Nonebefore sales startplace_renewal_orderonly works during Renewal phaseShared types (sp-coretime):
SalePhase,MarketError,OrderResult,RenewalOrderResult