[cu-33e4tdu] Attempt#2 Change order of execution of lock

[vimukthi-git]

Description

Changing order of locking to be after the balance has been updated with the new lock amount.

[itsbobbyzz]

Task linked: CU-33e4tdu Crowdloan rewards pallet - config changes (lock PICA)

[github-actions]

Nix commands for this PR

Make sure you have setup the Composable community cache:

(you only need to run it once on your machine)

nix-shell -p cachix --command "cachix use composable-community"

Show all possible apps, shells and packages:

nix flake show "github:ComposableFi/composable/6d01ce17156b23a04ebf738473040d30528ef440 --allow-import-from-derivation

Run the Composable node alone:

nix run "github:ComposableFi/composable/6d01ce17156b23a04ebf738473040d30528ef440#composable-node" -L

Spin up a local devnet:

nix run "github:ComposableFi/composable/6d01ce17156b23a04ebf738473040d30528ef440#devnet" -L --option sandbox relaxed --show-trace

Spin up a local XCVM devnet:

nix run "github:ComposableFi/composable/6d01ce17156b23a04ebf738473040d30528ef440#devnet-xcvm-up" -L

Are you on macOS, or do you not have Nix installed? No worries, you can also run these commands in Docker like this:

(you only need to run the first command once on your machine)

docker volume create nix

docker run --privileged --rm -v nix:/nix  -v /var/run/docker.sock:/var/run/docker.sock -it nixos/nix bash -c "nix-env -iA nixpkgs.cachix && cachix use composable-community && nix run github:ComposableFi/composable/6d01ce17156b23a04ebf738473040d30528ef440#devnet-up -L --option cores 8 --extra-experimental-features nix-command --extra-experimental-features flakes"

Note that the initial build may take about one hour if it has not been cached by our CI yet. Once it is cached, builds should take about one minute. We currently do not provide build caches for ARM machines such as M1 Macs, but building on ARM is supported.

[codecov-commenter]

Codecov Report

Base: 21.40% // Head: 21.40% // Decreases project coverage by -0.00% ⚠️

Coverage data is based on head (78195b3) compared to base (67b82ea).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 78195b3 differs from pull request most recent head 6d01ce1. Consider uploading reports for the commit 6d01ce1 to get more accurate results

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2020      +/-   ##
==========================================
- Coverage   21.40%   21.40%   -0.01%     
==========================================
  Files         419      419              
  Lines      153158   153159       +1     
==========================================
  Hits        32789    32789              
- Misses     120369   120370       +1     

Impacted Files	Coverage Δ
code/parachain/frame/crowdloan-rewards/src/lib.rs	54.68% <100.00%> (+0.10%)	⬆️
code/utils/price-feed/src/backend.rs	51.04% <0.00%> (-0.70%)	⬇️
code/parachain/frame/pablo/src/uniswap.rs	98.63% <0.00%> (-0.69%)	⬇️
code/parachain/frame/airdrop/src/lib.rs	55.18% <0.00%> (ø)
code/parachain/frame/staking-rewards/src/lib.rs	62.51% <0.00%> (+0.08%)	⬆️
...chain/frame/composable-traits/src/lending/tests.rs	99.43% <0.00%> (+0.56%)	⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[PoisonPhang]

🎉

[PoisonPhang]

From ClickUp

All the above scenarios were tested several time and I can confirm that the balance acting as locked now.

A new issue has arise now: - after the fist claiming, the new ones require native token that is free to pay GAS in the user's balance. This will block users to claim forward so wo need to address this behavior.

We either need this Tx to be Pays::No by default, or we can't be locking the entirety of their native token. (Unless one of the lock types allows for locked funds to pay for gas).

[vimukthi-git]

@PoisonPhang As explained on slack,

Locks have scopes, initially we had the scope as reserve then all , now I changed it to transfer to allow fee payments. It's well thought out actually

[LyonSsS]

Re tested the behavior of this ticket: https://app.clickup.com/t/33e4tdu using the current branch locally. All the issues were fixed and this PR can now be merged.

[vimukthi-git]

@Mergifyio refresh

[mergify]

refresh

✅ Pull request refreshed