Skip to content
This repository has been archived by the owner on Jul 27, 2022. It is now read-only.

Problem:(CRO-479) no HD wallet support in client-rpc #486

Merged
merged 1 commit into from
Oct 22, 2019

Conversation

leejw51crypto
Copy link
Collaborator

Solution: add hdwallet api to client-rpc

Comment on lines 28 to 31
#[rpc(name = "wallet_create_hd")]
fn create_hd(&self, request: WalletRequest) -> Result<String>;
Copy link
Collaborator

@devashishdxt devashishdxt Oct 18, 2019

Choose a reason for hiding this comment

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

Can we update current wallet_create RPC call to take WalletType as an additional parameter instead of creating a new RPC method?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ok

@codecov
Copy link

codecov bot commented Oct 18, 2019

Codecov Report

Merging #486 into master will increase coverage by 0.63%.
The diff coverage is 95.53%.

@@            Coverage Diff             @@
##           master     #486      +/-   ##
==========================================
+ Coverage   66.34%   66.98%   +0.63%     
==========================================
  Files         120      120              
  Lines       13909    14008      +99     
==========================================
+ Hits         9228     9383     +155     
+ Misses       4681     4625      -56
Impacted Files Coverage Δ
client-core/src/types/wallet_kind.rs 7.69% <100%> (-0.65%) ⬇️
client-rpc/src/rpc/wallet_rpc.rs 73.68% <95.49%> (+16.87%) ⬆️
client-core/src/service/key_service.rs 92.15% <0%> (+1.47%) ⬆️
client-common/src/key/public_key.rs 78.89% <0%> (+2.75%) ⬆️
client-core/src/wallet/default_wallet_client.rs 54.45% <0%> (+8.91%) ⬆️
client-core/src/input_selection.rs 72.72% <0%> (+72.72%) ⬆️

client-rpc/src/rpc/wallet_rpc.rs Outdated Show resolved Hide resolved
@tomtau
Copy link
Contributor

tomtau commented Oct 19, 2019

bors r+

bors bot added a commit that referenced this pull request Oct 19, 2019
486: Problem:(CRO-479) no HD wallet support in client-rpc r=tomtau a=leejw51crypto

Solution: add hdwallet api to client-rpc


Co-authored-by: Jongwhan Lee <jonghwan@crypto.com>
@bors
Copy link
Contributor

bors bot commented Oct 19, 2019

Build failed

@tomtau tomtau self-requested a review October 20, 2019 06:07
Copy link
Contributor

@tomtau tomtau left a comment

Choose a reason for hiding this comment

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

integration test doesn't pass -- probably needs some fixes?

@leejw51crypto
Copy link
Collaborator Author

it's lack of unit test coverage.
i'll add more unit test

@tomtau
Copy link
Contributor

tomtau commented Oct 21, 2019

@leejw51 @leejw51crypto the existing tests don't pass: https://travis-ci.org/crypto-com/chain/jobs/599922522#L5135

 1) Staking
5138       unbond of same amount and nonce from different account should have different txid:
5139     Invalid params: invalid length 1, expected a tuple of size 2.
5140  
5141
5142  2) Staking
5143       should support staking, unbonding and withdrawing:
5144     Invalid params: invalid length 1, expected a tuple of size 2.
5145  
5146
5147  3) Wallet Auto-sync
5148       can auto-sync unlocked wallets:
5149     Error: Error when creating receiver wallet: Invalid params: invalid length 1, expected a tuple of size 2.
5150      at Object.exports.asyncMiddleman (test/core/utils.ts:81:9)
5151      at process._tickCallback (internal/process/next_tick.js:68:7)
5152
5153  4) Wallet management
5154       can create wallet with specified name:
5155     Invalid params: invalid length 1, expected a tuple of size 2.
5156  
5157
5158  5) Wallet management
5159       Newly created wallet has a staking and transfer address associated:
5160     Invalid params: invalid length 1, expected a tuple of size 2.
5161  
5162
5163  6) Wallet management
5164       cannot create duplicated wallet:
5165     Invalid params: invalid length 1, expected a tuple of size 2.
5166  
5167
5168  7) Wallet management
5169       User cannot access wallet with incorrect passphrase:
5170     Invalid params: invalid length 1, expected a tuple of size 2.
5171  
5172
5173  8) Wallet management
5174       Create a transfer address and then list it:
5175     Invalid params: invalid length 1, expected a tuple of size 2.
5176  
5177
5178  9) Wallet management
5179       Create a staking address and then list it:
5180     Invalid params: invalid length 1, expected a tuple of size 2.
5181  
5182
5183  10) Wallet transaction
5184       Zero Fee
5185         can transfer funds between two wallets:
5186     Error: Error when creating receiver wallet: Invalid params: invalid length 1, expected a tuple of size 2.
5187      at Object.exports.asyncMiddleman (test/core/utils.ts:81:9)
5188      at process._tickCallback (internal/process/next_tick.js:68:7)
5189
5190  11) Wallet transaction
5191       With Fee
5192         can transfer funds between two wallets with fee included:
5193     Error: Error when creating receive wallet: Invalid params: invalid length 1, expected a tuple of size 2.
5194      at Object.exports.asyncMiddleman (test/core/utils.ts:81:9)
5195      at process._tickCallback (internal/process/next_tick.js:68:7)

@calvinaco

@leejw51crypto leejw51crypto force-pushed the cro-479 branch 3 times, most recently from 8b36b49 to ef0b13f Compare October 21, 2019 04:27
@leejw51crypto
Copy link
Collaborator Author

i'm working on intergration test

fix hdwallet creation


tidy up


replace String with SecUtf8


fix unit test


add unit test


add unit test for send amount


fix integration test
@leejw51
Copy link
Contributor

leejw51 commented Oct 22, 2019

fix integration test , giving wallet type "Basic" in calling "create wallet" like this

withFeeRpcClient.request("wallet_create", [receiverWalletRequest,"Basic"])

i added

"Basic"

thanks @calvinaco

@tomtau
Copy link
Contributor

tomtau commented Oct 22, 2019

bors try

bors bot added a commit that referenced this pull request Oct 22, 2019
@bors
Copy link
Contributor

bors bot commented Oct 22, 2019

@tomtau
Copy link
Contributor

tomtau commented Oct 22, 2019

bors r+

bors bot added a commit that referenced this pull request Oct 22, 2019
486: Problem:(CRO-479) no HD wallet support in client-rpc r=tomtau a=leejw51crypto

Solution: add hdwallet api to client-rpc


501: Problem: unrestricted drone exec pipeline r=tomtau a=tomtau

Solution: added trigger conditions, such that
"exec" pipelines are only executed after bors actions

Co-authored-by: Jongwhan Lee <jonghwan@crypto.com>
Co-authored-by: Tomas Tauber <2410580+tomtau@users.noreply.github.com>
@bors
Copy link
Contributor

bors bot commented Oct 22, 2019

@bors bors bot merged commit 81d0a55 into crypto-com:master Oct 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants