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

Problem (Fix #1621): Can't deposit into other's new staking address #1625

Merged
merged 1 commit into from
May 20, 2020

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented May 20, 2020

Solution:

  • Remove the restriction from client network ops code
  • Add check and double confirmation in client-cli

@codecov
Copy link

codecov bot commented May 20, 2020

Codecov Report

Merging #1625 into master will decrease coverage by 2.19%.
The diff coverage is 15.15%.

@@            Coverage Diff             @@
##           master    #1625      +/-   ##
==========================================
- Coverage   70.31%   68.12%   -2.20%     
==========================================
  Files         168      170       +2     
  Lines       21548    22247     +699     
==========================================
+ Hits        15151    15155       +4     
- Misses       6397     7092     +695     
Impacted Files Coverage Δ
client-cli/src/command/transaction_command.rs 0.00% <0.00%> (ø)
client-network/src/network_ops.rs 50.00% <33.33%> (ø)
...work/src/network_ops/default_network_ops_client.rs 75.04% <100.00%> (+0.64%) ⬆️
client-rpc/src/rpc/multisig_rpc.rs 37.91% <0.00%> (-23.70%) ⬇️
client-rpc/src/rpc/wallet_rpc.rs 67.09% <0.00%> (-13.32%) ⬇️
client-rpc/src/rpc/transaction_rpc.rs 78.84% <0.00%> (-6.58%) ⬇️
client-common/src/key/public_key.rs 78.86% <0.00%> (-6.23%) ⬇️
client-core/src/wallet/default_wallet_client.rs 49.27% <0.00%> (-5.99%) ⬇️
client-core/src/cipher/mock.rs 60.00% <0.00%> (-5.22%) ⬇️
...ient-core/src/service/multi_sig_session_service.rs 90.00% <0.00%> (-3.75%) ⬇️
... and 16 more

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.

maybe also bump version numbers in the affected client-* Cargo.toml

Comment on lines 145 to 151
let staked_state = self.get_staked_state(name, &to_address, verify_staking)?;
verify_unjailed(&staked_state).map_err(|e| {
Error::new(
ErrorKind::ValidationError,
format!("Failed to validate staking account: {}", e),
)
})?;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe the jailing test could be optionally run (if the state exists?)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

@yihuang yihuang force-pushed the issue1621 branch 2 times, most recently from 40bd70e to 335ceb6 Compare May 20, 2020 04:39
@yihuang
Copy link
Collaborator Author

yihuang commented May 20, 2020

maybe also bump version numbers in the affected client-* Cargo.toml

Done

@tomtau
Copy link
Contributor

tomtau commented May 20, 2020

maybe also bump version numbers in the affected client-* Cargo.toml

Done

client-rpc is also affected if client-network changed?

… address

Solution:
- Remove the restriction from client network ops code
- Add check and double confirmation in client-cli
@yihuang
Copy link
Collaborator Author

yihuang commented May 20, 2020

maybe also bump version numbers in the affected client-* Cargo.toml

Done

client-rpc is also affected if client-network changed?

Yes, just bumped the version too, and reflected that in CHANGELOG.

@tomtau
Copy link
Contributor

tomtau commented May 20, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented May 20, 2020

Build succeeded:

@bors bors bot merged commit 5e4ff7f into crypto-com:master May 20, 2020
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.

None yet

2 participants