added flag to track the withdrawl of fund for milestones #257

mshankarrao · 2023-10-31T04:05:42Z

Added the flag in the milestone for tracking the withdrawl which is set to true once the withrawl has been done for the given milestone
Added the payment address for the project to allow user to request payment in a given address type

f-gate

Drop me a message so we can talk about how to next proceed!

f-gate · 2023-10-31T07:48:12Z

pallets/proposals/src/migration.rs

@@ -45,6 +46,7 @@ mod v0 {
        pub approved_for_funding: bool,
        pub funding_threshold_met: bool,
        pub cancelled: bool,
+        pub payment_address: [u8;20],


Why are you adding payment_address to old migration types?

f-gate · 2023-10-31T07:49:21Z

pallets/proposals/src/migration.rs

@@ -19,6 +19,7 @@ mod v0 {
        pub name: Vec<u8>,
        pub percentage_to_unlock: u32,
        pub is_approved: bool,
+        pub withdrawn: bool,


pls remove this line its an old migration type, it should be preserved and there is no reason to change it as we are on v7 or something now.

f-gate · 2023-10-31T07:49:30Z

pallets/proposals/src/migration.rs

@@ -74,6 +76,7 @@ mod v1 {
        pub approved_for_funding: bool,
        pub funding_threshold_met: bool,
        pub cancelled: bool,
+        pub payment_address: [u8;20],


f-gate · 2023-10-31T07:49:39Z

pallets/proposals/src/migration.rs

@@ -168,6 +172,7 @@ mod v2 {
        pub funding_threshold_met: bool,
        pub cancelled: bool,
        pub funding_type: FundingType,
+        pub payment_address: [u8;20],


f-gate · 2023-10-31T07:51:13Z

pallets/proposals/src/migration.rs

@@ -176,6 +181,7 @@ mod v2 {
        pub milestone_key: MilestoneKey,
        pub percentage_to_unlock: u32,
        pub is_approved: bool,
+        pub withdrawn: bool,


Nothing in mod v2 should be changed, in fact nothing in any migration should be changed except possibly the latest.
Feel free to drop me a message if you wanna chat about migrations and steps for introducing your new field.

f-gate · 2023-10-31T07:56:38Z

pallets/proposals/src/impls/pallet_impls.rs

+                    if ms.is_approved {
+                        ms.withdrawn = true;
+                    }
+                }
                if p.withdrawn_funds == p.raised_funds {


This is nice, We can now remove this possibly troublesome line if p.withdrawn_funds == p.raised_funds { which doesnt handle with dust at all and replace it with something like.

if milestones.iter().all(|&ms|{ ms.withdrawn }) {

f-gate · 2023-10-31T08:08:40Z

pallets/proposals/src/lib.rs

@@ -562,6 +565,7 @@ pub struct Project<T: Config> {
    pub cancelled: bool,
    pub funding_type: FundingType,
    pub deposit_id: DepositIdOf<T>,
+    pub payment_address: [u8; 20],


This is meant to signal where the tokens will end up. check telegram when you get a minute!

samelamin · 2023-11-13T15:43:32Z

pallets/proposals/src/migration.rs

@@ -273,6 +273,7 @@ pub mod v3 {
                            milestone.percentage_to_unlock as u8,
                        ),
                        is_approved: milestone.is_approved,
+                        withdrawn: milestone.withdrawn


how will these migrations work, the old data doesnt have these flags. You can default them is_approved for now

samelamin · 2023-11-13T15:43:47Z

pallets/proposals/src/migration.rs

@@ -296,6 +297,7 @@ pub mod v3 {
                        funding_type: FundingType::Proposal,
                        // A deposit_id of u32::MAX is ignored.
                        deposit_id: u32::MAX.into(),
+                        payment_address: project.payment_address,


this should default to the initiator address

samelamin · 2023-11-13T15:44:57Z

@mshankarrao we need to pass in the payment address when we create the project, I dont see it in the current pr?

we need to pass the payment address when we create the project, either through a brief or a grant.

We can have it as an optional parameter and if it's a foreign currency and paymentAddress isnt passed we should have an ensure that throws an error

Also if it isn't a foreign currency then we should default it to the project owners address

f-gate · 2023-11-21T09:24:04Z

pallets/proposals/src/migration.rs

+                agreement_hash: Default::default(),
+                cancelled: project.cancelled,
+                raised_funds: project.raised_funds,
+                funding_type: FundingType::Proposal,


use the funding type of the old project instead of overwiting with FundingType::Proposal.

f-gate · 2023-11-21T09:24:51Z

pallets/proposals/src/migration.rs

+                withdrawn_funds: project.withdrawn_funds,
+                initiator: project.initiator,
+                created_on: project.created_on,
+                agreement_hash: Default::default(),


use the agreement_hash of the old project instead of overwriting!!!

f-gate · 2023-11-21T09:26:54Z

pallets/proposals/src/migration.rs

+        pub withdrawn: bool,
+    }
+    #[derive(Encode, Decode, Clone)]
+    pub struct ProjectV7<AccountId, Balance, BlockNumber> {


This is meant to be the old Project type, the new project type is not defined here but defined in crate::Project
This should be the Project before you made the changes.

f-gate · 2023-11-21T09:27:21Z

pallets/proposals/src/migration.rs

+    {
+        let mut weight = T::DbWeight::get().reads_writes(1, 1);
+        let mut migrated_milestones: BTreeMap<MilestoneKey, MilestoneV7> = BTreeMap::new();
+        v7::Projects::<T>::translate(|_project_key, project: ProjectV7Of<T>| {


If you use v7::Projects::drain its much easier to understand

samelamin · 2023-12-05T10:45:55Z

@f-gate this pr is no longer needed correct ?

f-gate · 2023-12-05T11:00:16Z

No longer needed fix merged with #233

added flag to track the withdrawl of fund for milestones

4e9871c

mshankarrao requested review from samelamin and f-gate October 31, 2023 04:05

f-gate requested changes Oct 31, 2023

View reviewed changes

mshankarrao added 2 commits October 31, 2023 21:29

Merge branch 'main' into flag_to_track_milestone_withdrawl

0d03460

changed the migrations files based on the review comments

43fbd76

samelamin reviewed Nov 13, 2023

View reviewed changes

mshankarrao added 2 commits November 13, 2023 22:47

Merge branch 'main' into flag_to_track_milestone_withdrawl

86360ca

added the migration for adding extra field into the milestone

ed6fc1b

f-gate requested changes Nov 21, 2023

View reviewed changes

mapped the milestone and project upon migration to the currect version

9a0f26a

f-gate closed this Dec 5, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added flag to track the withdrawl of fund for milestones #257

added flag to track the withdrawl of fund for milestones #257

mshankarrao commented Oct 31, 2023

f-gate left a comment

f-gate Oct 31, 2023

f-gate Oct 31, 2023

f-gate Oct 31, 2023

mshankarrao Nov 1, 2023

f-gate Oct 31, 2023

mshankarrao Nov 1, 2023

f-gate Oct 31, 2023

f-gate Oct 31, 2023

samelamin Nov 13, 2023

f-gate Oct 31, 2023

mshankarrao Nov 1, 2023

samelamin Nov 13, 2023

samelamin Nov 13, 2023

samelamin commented Nov 13, 2023 •

edited

Loading

f-gate Nov 21, 2023

f-gate Nov 21, 2023

f-gate Nov 21, 2023

f-gate Nov 21, 2023 •

edited

Loading

samelamin commented Dec 5, 2023

f-gate commented Dec 5, 2023

added flag to track the withdrawl of fund for milestones #257

added flag to track the withdrawl of fund for milestones #257

Conversation

mshankarrao commented Oct 31, 2023

f-gate left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samelamin commented Nov 13, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

f-gate Nov 21, 2023 • edited Loading

Choose a reason for hiding this comment

samelamin commented Dec 5, 2023

f-gate commented Dec 5, 2023

samelamin commented Nov 13, 2023 •

edited

Loading

f-gate Nov 21, 2023 •

edited

Loading