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

Problem (Fix #949): deposit amount fee calculation is not correct #1118

Merged
merged 1 commit into from Feb 24, 2020
Merged

Problem (Fix #949): deposit amount fee calculation is not correct #1118

merged 1 commit into from Feb 24, 2020

Conversation

yihuang
Copy link
Collaborator

@yihuang yihuang commented Feb 21, 2020 •
edited

Solution:

tomtau
tomtau suggested changes Feb 21, 2020
View reviewed changes
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.

also there's a fee computation function in c bindings

client-core/src/signer/dummy_signer.rs Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Feb 21, 2020 •
edited

Codecov Report

Merging #1118 into master will decrease coverage by 1.21%.
The diff coverage is 0.27%.

@@            Coverage Diff             @@
##           master    #1118      +/-   ##
==========================================
- Coverage   65.57%   64.36%   -1.22%     
==========================================
  Files         145      148       +3     
  Lines       19041    19393     +352     
==========================================
- Hits        12487    12483       -4     
- Misses       6554     6910     +356
Impacted Files Coverage Δ
cro-clib/src/address.rs 0% <ø> (ø) ⬆️
cro-clib/src/transaction_staking.rs 0% <0%> (ø)
cro-clib/src/fee.rs 0% <0%> (ø) ⬆️
chain-abci/src/enclave_bridge/mock.rs 82.1% <0%> (-5.54%) ⬇️
cro-clib/src/transaction.rs 0% <0%> (ø)
cro-clib/src/basic_wallet.rs 0% <0%> (ø) ⬆️
cro-clib/src/transaction_build.rs 0% <0%> (ø) ⬆️
cro-clib/src/wallet.rs 0% <0%> (ø) ⬆️
cro-clib/src/transaction_deposit.rs 0% <0%> (ø)
cro-clib/src/types.rs 0% <0%> (ø) ⬆️
... and 7 more

@yihuang
Copy link
Collaborator Author

yihuang commented Feb 21, 2020

also there's a fee computation function in c bindings

cro_estimate_fee? that one needs bigger change because it only takes a tx_payload_size as an argument, which I think it's the size of the tx before obfuscation, that's not enough to compute the fee.

@yihuang
Problem (Fix crypto-com#949): deposit amount fee calculation is not c…
a343137 
…orrect

Solution:
- Fix DummySigner for deposit transaction
- Fix mock_enc_dec payload padding
- Test withdraw with viewkeys in the multinode join test (Fix crypto-com#1101)
@yihuang
Copy link
Collaborator Author

yihuang commented Feb 21, 2020 •
edited

#1101 is also solved by this, it's verified in the multinode join test, because normal integration tests will withdraw all at the beginning.

@tomtau
Copy link
Contributor

tomtau commented Feb 22, 2020

bors try

bors bot added a commit that referenced this pull request Feb 22, 2020
@bors
Try #1118:
e7ff9e1
@bors
Copy link
Contributor

bors bot commented Feb 22, 2020

try

Build succeeded

@yihuang yihuang force-pushed the issue949 branch 3 times, most recently from c28a50f to a343137 Compare February 22, 2020 09:40
@tomtau
Copy link
Contributor

tomtau commented Feb 24, 2020

bors r+

@bors
Copy link
Contributor

bors bot commented Feb 24, 2020

Build succeeded

@bors bors bot merged commit 18eafc2 into crypto-com:master Feb 24, 2020
@yihuang yihuang deleted the issue949 branch February 24, 2020 01:39
@tomtau tomtau mentioned this pull request Feb 24, 2020
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@tomtau tomtau tomtau requested changes

@calvinaco calvinaco Awaiting requested review from calvinaco calvinaco is a code owner

@devashishdxt devashishdxt Awaiting requested review from devashishdxt devashishdxt is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Problem: withdraw all coins estimates a wrong fee when extra view keys are entered
2 participants
@yihuang @tomtau