Skip to content

Luxor/vested wg spending#5072

Merged
mnaamani merged 40 commits intoJoystream:luxorfrom
dobertRowneySr:luxor/vested-wg-spending
Apr 3, 2024
Merged

Luxor/vested wg spending#5072
mnaamani merged 40 commits intoJoystream:luxorfrom
dobertRowneySr:luxor/vested-wg-spending

Conversation

@dobertRowneySr
Copy link
Copy Markdown
Collaborator

@dobertRowneySr dobertRowneySr commented Feb 5, 2024

Addresses: #4948
based on top of #5062

Note: I have used the extrinsic vested_transfer inside the extrinsic spend_from_budget all extrinsic in substrate are now transactional by default, this means that if the vested_transfer fails then spend_from_budget is equivalent to a no-op.
Also I have created one internal account for each WG where tokens are minted before they are transferred with vesting.

@dobertRowneySr
Copy link
Copy Markdown
Collaborator Author

To be rebased onto #5062

@mnaamani
Copy link
Copy Markdown
Member

I think there is a simpler way to do this, without needing to have an account on each working group.
Instead of pay_from_budget into the working group account then doing vesting::vested_transfer you should be able to pay from the budget directly into the target account, then do vesting::force_vested_transfer with source and target being the final destination. The balances::transfer inside force_vested_transfer will be a successful no-op, and then the vesting schedule will be applied.

@mnaamani mnaamani self-requested a review March 16, 2024 22:38
@mnaamani
Copy link
Copy Markdown
Member

Also I have created one internal account for each WG where tokens are minted before they are transferred with vesting.

#5072 (comment)

@mnaamani
Copy link
Copy Markdown
Member

I think this now needs to be rebased on luxor since I merged the reduce council budget PR

Copy link
Copy Markdown
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

some additional cleanup for build to works after refactor (removing the moduleId in working groups)

@dobertRowneySr dobertRowneySr force-pushed the luxor/vested-wg-spending branch from 0cf5f79 to ac4b0b1 Compare March 28, 2024 07:21
@kdembler kdembler requested a review from mnaamani March 29, 2024 09:59
Copy link
Copy Markdown
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

Benchmark is failing

Error: Input("Benchmark working_group::vested_spend_from_budget failed: AmountLow")
2024-03-29 22:54:25 Starting benchmark: working_group::vested_spend_from_budget    
There was a problem generating the weights for working_group, check the error above
Error: Process completed with exit code 1.

None
);

let amount_vesting = VestingBalanceOf::<T>::from(10_000u32);
Copy link
Copy Markdown
Member

@mnaamani mnaamani Mar 30, 2024

Choose a reason for hiding this comment

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

Benchmark is failing with error hereAmountLow because amount is less than vesting pallet configuration of <T as vesting::Config>::MinVestedTransfer::get()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So perhaps use that constant to make the amount_vesting at least that amount.

Copy link
Copy Markdown
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

from earlier

None
);

let amount_vesting = VestingBalanceOf::<T>::from(10_000u32);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

So perhaps use that constant to make the amount_vesting at least that amount.


let amount_vesting = VestingBalanceOf::<T>::from(10_000u32);
let budget = BalanceOf::<T>::from(100_000u32);
let block_no = 100u32;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

block_no is not used

@mnaamani mnaamani self-requested a review April 2, 2024 17:34
@mnaamani
Copy link
Copy Markdown
Member

mnaamani commented Apr 3, 2024

strange that I'm seeing errors like ../../query-node/start.sh: line 11: docker-compose: command not found in CI workflow runs..
Has it been deprecated and we should replace all occurrences of that command with docker compose command?

I think you forgot to commit changes to: types/src/augment/augment-api-rpc.ts and types/src/augment/augment-types.ts

Copy link
Copy Markdown
Member

@mnaamani mnaamani left a comment

Choose a reason for hiding this comment

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

LGTM, next PR to luxor should commit changes to types/src/*.rs missed in this PR

Running integration tests locally on my machine works, we need to investigate why they are failing on github runners.

@mnaamani mnaamani merged commit aed16d0 into Joystream:luxor Apr 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants