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

Merge request pipeline #39523 passed

Stage: labels
    Stage: quality
      Stage: build
        Stage: tests
          Stage: deploy

            Merge request pipeline passed for 681c10cc

            Approved by
            Hugo Trentesaux

            Merged by Pascal EngélibertPascal Engélibert 4 months ago (Jan 14, 2025 2:49pm UTC)

            Merge details

            • Changes merged into master with 33060a55 (commits were squashed).
            • Deleted the source branch.

            • Closed #273 (closed)

            • Auto-merge enabled

            Pipeline #39524 passed

            Stage: build
              Stage: tests
                Stage: deploy

                  Pipeline passed for 33060a55 on master

                  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