Skip to content
Snippets Groups Projects

Disable UD for non-members at genesis

Merged Disable UD for non-members at genesis
tuxmain/genesis-idty-ud into master
3 unresolved threads
Merged Pascal Engélibert requested to merge tuxmain/genesis-idty-ud into master
3 unresolved threads
  • Add end2end test about UD eligibility at genesis for members or non-members
  • Fix non-eligibility for non-members
  • Remove implicit IdtyData::new
  • Make FirstEligibleUd::default explicit in doc

fixes #273 (closed)

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
node/src/chain_spec/gdev.rs
286 286 index: idty_index,
287 287 name: common_runtime::IdtyName::from(name.as_str()),
288 288 value: common_runtime::IdtyValue {
289 data: IdtyData::new(),
289 data: IdtyData {
290 first_eligible_ud: match status {
291 common_runtime::IdtyStatus::Member => 1_u16.into(),
292 _ => gdev_runtime::pallet_universal_dividend::FirstEligibleUd(None),
  • Comment on lines +291 to +292
    Hugo Trentesaux
    Hugo Trentesaux @HugoTrentesaux ·
    Owner

    We could add a comment like this:

    Suggested change
    293 common_runtime::IdtyStatus::Member => 1_u16.into(),
    294 _ => gdev_runtime::pallet_universal_dividend::FirstEligibleUd(None),
    293 // only members are eligible to UD claim
    294 // the first claimable UD is the first UD (number 1)
    295 common_runtime::IdtyStatus::Member => 1_u16.into(),
    296 _ => gdev_runtime::pallet_universal_dividend::FirstEligibleUd(None),

    And we could also use FirstEligibleUd::min() which looks less arbitrary.

  • Pascal Engélibert
    Pascal Engélibert @tuxmain ·
    Author Maintainer

    I used Into because it's shorter but yes min is better.

  • Please register or sign in to reply
  • Hugo Trentesaux
    Hugo Trentesaux @HugoTrentesaux started a thread on the diff
    • pallets/universal-dividend/src/types.rs
    22 22 pub type UdIndex = u16;
    23 23
    24 24 /// Represents the first eligible Universal Dividend.
    25 #[derive(Clone, Default, Eq, PartialEq, RuntimeDebug, serde::Deserialize, serde::Serialize)]
    25 #[derive(Clone, Eq, PartialEq, RuntimeDebug, serde::Deserialize, serde::Serialize)]
    26 26 pub struct FirstEligibleUd(pub Option<NonZeroU16>);
    27 27
    28 /// Default is not eligible
    29 impl Default for FirstEligibleUd {
    30 fn default() -> Self {
    31 FirstEligibleUd(None)
    32 }
    33 }
  • Hugo Trentesaux
    Hugo Trentesaux @HugoTrentesaux started a thread on an old version of the diff
    • runtime/gdev/tests/common/mod.rs
    223 223 index: i as u32 + 1,
    224 224 name: name.clone(),
    225 225 value: IdtyValue {
    226 data: IdtyData::new(),
    226 data: IdtyData {
    227 first_eligible_ud: 1_u16.into(),
  • Hugo Trentesaux
    Hugo Trentesaux @HugoTrentesaux ·
    Owner

    Nice. For more coherence, we have to chose between

    • remove FirstEligibleUd::min() implementation
    • use it
  • Hugo Trentesaux approved this merge request

    approved this merge request

  • Pascal Engélibert added 4 commits

    added 4 commits

    • b5630b24 - 1 commit from branch master
    • 3f3990d9 - end2end: non-member idty has UD at genesis
    • 1f18c5bc - Fix UD eligibility at genesis
    • 681c10cc - use FirstEligibleUd::min

    Compare with previous version

    Toggle commit list
  • Pascal Engélibert changed time estimate to 2h 20m from 2h

    changed time estimate to 2h 20m from 2h

  • Pascal Engélibert enabled an automatic merge when the pipeline for 681c10cc succeeds

    enabled an automatic merge when the pipeline for 681c10cc succeeds

  • Pascal Engélibert merged

    merged

  • Pascal Engélibert mentioned in commit 33060a55

    mentioned in commit 33060a55

    • Please register or sign in to reply