Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CSUB-556: Add Status check to Withdraw Unbonded command #1149

Merged
merged 6 commits into from
Jun 22, 2023

Conversation

pLabarta
Copy link
Contributor

Description of proposed changes

  • Added canWithdraw field to Status
  • Reformatted how printStatus shows unlocked chunks
  • withdraw-unbonded command now checks if there are unlocked funds before submitting the extrinstic

Practical tips for PR review & merge:

  • All GitHub Actions report PASS
  • Newly added code/functions have unit tests
    • Coverage tools report all newly added lines as covered
    • The positive scenario is exercised
    • Negative scenarios are exercised, e.g. assert on all possible errors
    • Assert on events triggered if applicable
    • Assert on changes made to storage if applicable
  • Modified behavior/functions - try to make sure above test items are covered
  • Integration tests are added if applicable/needed

@pLabarta
Copy link
Contributor Author

This PR is based on #1148

@codecov-commenter
Copy link

codecov-commenter commented Jun 20, 2023

Codecov Report

Merging #1149 (e0a8672) into dev (211e68a) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##              dev    #1149   +/-   ##
=======================================
  Coverage   74.62%   74.62%           
=======================================
  Files          74       74           
  Lines       10686    10686           
=======================================
  Hits         7974     7974           
  Misses       2712     2712           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@atodorov atodorov self-requested a review June 21, 2023 16:53
@github-actions
Copy link

For full LLVM coverage report click here!

@nathanwhit nathanwhit force-pushed the cli-withdraw-checks branch 2 times, most recently from a0ea22a to d0beb47 Compare June 22, 2023 18:05
@nathanwhit nathanwhit marked this pull request as ready for review June 22, 2023 18:10
Copy link
Contributor

@atodorov atodorov left a comment

Choose a reason for hiding this comment

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

with V3_CONTROL - not bonded, no unlocked funds so looks good:

$ node dist/index.js withdraw-unbonded  -u ws://127.0.0.1:9946 
2023-06-22 21:16:32        API/INIT: RPC methods not decorated: creditcoin_hashrate, task_getOffchainNonceKey
2023-06-22 21:16:32        API/INIT: creditcoin-node/222: Not decorating unknown runtime apis: TaskApi/1
✔ Specify a seed phrase for the Controller account … *******************************************************************************
Cannot perform action, there are no unlocked funds to withdraw

V2_CONTROL - currently bonded, there are 3,860 CTC currently unlocked but the validator is still validating.

image

[gluwa@fedora cc-cli]$ node dist/index.js status -u ws://127.0.0.1:9946 -a 5FEK97W73jQHWdUYAeZvyb7iRuygZhFfC75ZA9cfqkQ8yhvb
2023-06-22 21:20:22        API/INIT: RPC methods not decorated: creditcoin_hashrate, task_getOffchainNonceKey
2023-06-22 21:20:22        API/INIT: creditcoin-node/222: Not decorating unknown runtime apis: TaskApi/1
Bonded:  true
Stash:  undefined
Controller:  5EEoJZYPpTYGX1t3shaQbqhRmqvdvoaTycY1gJ4TB8spDgPp
Validating:  true
Waiting:  false
Active:  true
Can withdraw:  true
Unlocked chunks: 
  929 CTC unlocked since era 615
  930 CTC unlocked since era 616
  2000 CTC unlocked since era 618
Next unbonding chunk: None



[gluwa@fedora cc-cli]$ node dist/index.js show-address -u ws://127.0.0.1:9946
✔ Specify caller's seed phrase … ****************************************************************************
Account address: 5EEoJZYPpTYGX1t3shaQbqhRmqvdvoaTycY1gJ4TB8spDgPp

^^^ pasted the conroller seed. Address matched above


[gluwa@fedora cc-cli]$ node dist/index.js withdraw-unbonded  -u ws://127.0.0.1:9946 
2023-06-22 21:20:45        API/INIT: RPC methods not decorated: creditcoin_hashrate, task_getOffchainNonceKey
2023-06-22 21:20:45        API/INIT: creditcoin-node/222: Not decorating unknown runtime apis: TaskApi/1
✔ Specify a seed phrase for the Controller account … ****************************************************************************
Cannot perform action, there are no unlocked funds to withdraw

[gluwa@fedora cc-cli]$ echo $?
1

^^^ pasted the controller seed, but the command failed

UPDATE: this is resolved by the last commit!

@atodorov atodorov marked this pull request as draft June 22, 2023 18:23
@atodorov atodorov requested a review from jonZlotnik June 22, 2023 18:27
@atodorov atodorov marked this pull request as ready for review June 22, 2023 18:30
atodorov
atodorov previously approved these changes Jun 22, 2023
@nathanwhit nathanwhit enabled auto-merge (squash) June 22, 2023 18:55
AdaJane
AdaJane previously approved these changes Jun 22, 2023
@nathanwhit nathanwhit merged commit 0bc421f into dev Jun 22, 2023
28 of 29 checks passed
@nathanwhit nathanwhit deleted the cli-withdraw-checks branch June 22, 2023 20:18
AdaJane added a commit that referenced this pull request Jun 26, 2023
* add: canWithdraw to status

* add: reformat how unlocked chunks are printed

* add: withdraw command checks unlocked funds

* Appease the linter

* Check stash status in withdrawUnbondedAction

---------

Co-authored-by: Ada Anderson <ada@somepanic.com>
Co-authored-by: Nathan Whitaker <nathan.whitaker01@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants