Problem: (CRO-294) Client can generate invalid transactions by devashishdxt · Pull Request #537 · crypto-com/thaler

devashishdxt · 2019-10-30T06:04:02Z

Solution: Added balance checks when creating network ops transactions

codecov · 2019-10-30T06:39:08Z

Codecov Report

Merging #537 into master will decrease coverage by <.01%.
The diff coverage is 71.42%.

@@            Coverage Diff             @@
##           master     #537      +/-   ##
==========================================
- Coverage   67.23%   67.22%   -0.01%     
==========================================
  Files         122      122              
  Lines       14102    14129      +27     
==========================================
+ Hits         9481     9498      +17     
- Misses       4621     4631      +10

Impacted Files	Coverage Δ
client-cli/src/command.rs	`0% <0%> (ø)`	⬆️
client-core/src/service/wallet_state_service.rs	`89.17% <100%> (+0.28%)`	⬆️
client-core/src/wallet/default_wallet_client.rs	`59.12% <100%> (+0.82%)`	⬆️
...work/src/network_ops/default_network_ops_client.rs	`85.81% <65.21%> (-1.41%)`	⬇️
chain-core/src/tx/fee/mod.rs	`87.68% <0%> (-0.5%)`	⬇️

tomtau

looks ok, but is it only for staked state operations?
e.g. if one sends TX that spends some tx inputs... if the client doesn't sync, those inputs would still be "unspent" in the internal state (rather than "potentially spent" with some TXID to check whether it was really committed), is that correct?

devashishdxt · 2019-10-30T07:19:57Z

looks ok, but is it only for staked state operations?
e.g. if one sends TX that spends some tx inputs... if the client doesn't sync, those inputs would still be "unspent" in the internal state (rather than "potentially spent" with some TXID to check whether it was really committed), is that correct?

That is correct. Client won't know if some inputs are spent until it syncs the block in which they were spent.

tomtau · 2019-10-30T07:47:55Z

looks ok, but is it only for staked state operations?
e.g. if one sends TX that spends some tx inputs... if the client doesn't sync, those inputs would still be "unspent" in the internal state (rather than "potentially spent" with some TXID to check whether it was really committed), is that correct?

That is correct. Client won't know if some inputs are spent until it syncs the block in which they were spent.

ok, I created CRO-524 for that

tomtau · 2019-10-30T07:48:06Z

bors r+

calvinaco

Look good to me, leave a small suggestion about using view_key when checking wallet existence.

calvinaco · 2019-10-30T08:03:57Z

client-core/src/wallet/default_wallet_client.rs

+        inputs: &[TxoPointer],
+    ) -> Result<bool> {
+        // Check if wallet exists
+        self.wallet_service.view_key(name, passphrase)?;


A small suggestion: I notice we are using view_key to check for wallet existence or passphrase correctness in wallet client, maybe we can add a wallet_service.has_wallet() ?

535: Problem(CRO-392)outdated dependencies in client's storage encryption r=tomtau a=linfeng-crypto Solution: - use crate `aes-gcm-siv` and `aead` instead of `miscreant` - use crate `rust-argon2` to the `passphrase` to a constant length, and store the `salt` at the end of the encrypted data. 537: Problem: (CRO-294) Client can generate invalid transactions r=tomtau a=devashishdxt Solution: Added balance checks when creating network ops transactions 538: Problem:(CRO-521) Problem: unbonded from custom time is ignored in genesis initconfig r=tomtau a=linfeng-crypto Solution: change the parameters of `new_init`: change `genesis_time` from `Timespec` into `Option<Timespec>`, remove the `bool` type parameter `bonded`, add a `&StakedStateDestination` type parameter. Co-authored-by: ylf <cxwcylf@126.com> Co-authored-by: Devashish Dixit <devashish@crypto.com> Co-authored-by: linfeng <linfeng@crypto.com>

bors · 2019-10-30T09:23:59Z

Build failed (retrying...)

continuous-integration/travis-ci/push

leejw51crypto

ok

537: Problem: (CRO-294) Client can generate invalid transactions r=tomtau a=devashishdxt Solution: Added balance checks when creating network ops transactions Co-authored-by: Devashish Dixit <devashish@crypto.com>

devashishdxt · 2019-10-30T11:11:50Z

bors retry

bors · 2019-10-30T11:11:51Z

Already running a review

bors · 2019-10-30T11:38:55Z

Build failed

continuous-integration/drone/push

tomtau · 2019-10-31T01:31:59Z

bors retry

537: Problem: (CRO-294) Client can generate invalid transactions r=tomtau a=devashishdxt Solution: Added balance checks when creating network ops transactions 538: Problem:(CRO-521) Problem: unbonded from custom time is ignored in genesis initconfig r=tomtau a=linfeng-crypto Solution: change the parameters of `new_init`: change `genesis_time` from `Timespec` into `Option<Timespec>`, remove the `bool` type parameter `bonded`, add a `&StakedStateDestination` type parameter. 540: Problem: (CRO-501) Integration tests is not running in HW SGX r=tomtau a=calvinlauco Solution: Add integration tests in Drone CI running in HW SGX --- - Integration tests will run as SW mode in TravisCI and HW mode in Drone Co-authored-by: Devashish Dixit <devashish@crypto.com> Co-authored-by: linfeng <linfeng@crypto.com> Co-authored-by: Calvin Lau <calvinlauco@gmail.com>

bors · 2019-10-31T02:52:10Z

Build failed (retrying...)

continuous-integration/travis-ci/push

537: Problem: (CRO-294) Client can generate invalid transactions r=tomtau a=devashishdxt Solution: Added balance checks when creating network ops transactions Co-authored-by: Devashish Dixit <devashish@crypto.com>

tomtau · 2019-10-31T03:30:05Z

bors r+

537: Problem: (CRO-294) Client can generate invalid transactions r=tomtau a=devashishdxt Solution: Added balance checks when creating network ops transactions 542: Problem: app hash changes even if app state didn't change (CRO-526) r=tomtau a=tomtau Solution: added a flag to chainabci node to signal whether rewards pool was updated in that block (either by slashing events or transaction fees -- in the future reward distribution), so that its "last_block_height" (which denotes when it was updated last time) is only updated when the amount changed. TODO: perhaps move the rewards pool update logic to rewards pool code (rather than in abci)? Co-authored-by: Devashish Dixit <devashish@crypto.com> Co-authored-by: Tomas Tauber <2410580+tomtau@users.noreply.github.com>

bors · 2019-10-31T04:25:33Z

Build succeeded

Problem: (CRO-294) Client can generate invalid transactions

44abaad

Solution: Added balance checks when creating network ops transactions

devashishdxt requested review from calvinaco, leejw51crypto, lezzokafka, linfeng-crypto and tomtau October 30, 2019 06:04

tomtau approved these changes Oct 30, 2019

View reviewed changes

calvinaco approved these changes Oct 30, 2019

View reviewed changes

leejw51crypto approved these changes Oct 30, 2019

View reviewed changes

lezzokafka approved these changes Oct 31, 2019

View reviewed changes

bors bot merged commit 44abaad into crypto-com:master Oct 31, 2019

devashishdxt deleted the tx-validity branch October 31, 2019 07:09

Conversation

devashishdxt commented Oct 30, 2019

Uh oh!

codecov bot commented Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tomtau left a comment

Choose a reason for hiding this comment

Uh oh!

devashishdxt commented Oct 30, 2019

Uh oh!

tomtau commented Oct 30, 2019

Uh oh!

tomtau commented Oct 30, 2019

Uh oh!

calvinaco left a comment

Choose a reason for hiding this comment

Uh oh!

calvinaco Oct 30, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors bot commented Oct 30, 2019

Build failed (retrying...)

Uh oh!

leejw51crypto left a comment

Choose a reason for hiding this comment

Uh oh!

devashishdxt commented Oct 30, 2019

Uh oh!

bors bot commented Oct 30, 2019

Uh oh!

bors bot commented Oct 30, 2019

Build failed

Uh oh!

tomtau commented Oct 31, 2019

Uh oh!

bors bot commented Oct 31, 2019

Build failed (retrying...)

Uh oh!

tomtau commented Oct 31, 2019

Uh oh!

bors bot commented Oct 31, 2019

Build succeeded

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

codecov bot commented Oct 30, 2019 •

edited

Loading

calvinaco Oct 30, 2019 •

edited

Loading