Skip to content

Conversation

@jhtitor
Copy link
Contributor

@jhtitor jhtitor commented Apr 23, 2018

This should NOT be used instead of get_required_fees API call (although it should be possible in the future). The main purpose here is to aid Blind Transfers.

@codecov-io
Copy link

codecov-io commented Apr 23, 2018

Codecov Report

Merging #91 into develop will increase coverage by 7.68%.
The diff coverage is 7.4%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #91      +/-   ##
===========================================
+ Coverage    59.56%   67.25%   +7.68%     
===========================================
  Files           37       51      +14     
  Lines         3539     4113     +574     
===========================================
+ Hits          2108     2766     +658     
+ Misses        1431     1347      -84
Impacted Files Coverage Δ
bitshares/bitshares.py 43.51% <7.4%> (-2.83%) ⬇️
bitshares/account.py 63.46% <0%> (-23.08%) ⬇️
bitshares/committee.py 68.18% <0%> (-18.19%) ⬇️
bitshares/blockchainobject.py 87.85% <0%> (-5.96%) ⬇️
bitshares/utils.py 40% <0%> (-3.34%) ⬇️
bitshares/transactionbuilder.py 66.09% <0%> (-0.72%) ⬇️
bitsharesbase/operations.py 74.48% <0%> (-0.71%) ⬇️
bitsharesbase/memo.py 90.38% <0%> (-0.53%) ⬇️
bitshares/exceptions.py 100% <0%> (ø) ⬆️
... and 28 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb85138...bec15bd. Read the comment docs.

@xeroc
Copy link
Member

xeroc commented Apr 27, 2018

Why would you not always want to obtain the required fee from the API server? What's the usecase (other than removing one API call)?

@jhtitor
Copy link
Contributor Author

jhtitor commented Apr 27, 2018

  1. get_required_fees doesn't work correctly for Transfer_from_blind and Blind_transfer operations in bitshares-core. Instead a local fee calculator is used. And yes 5 * GRAPHENE_PRECISION is hard-coded, on the wallet side!

  2. When doing Transfer_from_blind, you have to include the fee INTO Blind_transfer amount, meaning you have to pre-calculate the fee for the operation, before it is fully constructed. This is not a serious issue, as long you only have to do it once, however...

  3. ...there is yet another bug/issue in bitshares-core, which forces you to precalculate this fee TWO times, for one and for two outputs, so we can actually avoid TWO round-trips to server.

Plus: I think it's nice to have such a feature. You cache fee definitions, and then you can re-calculate fees locally.

@jhtitor jhtitor mentioned this pull request May 8, 2018
6 tasks
@xeroc
Copy link
Member

xeroc commented May 28, 2018

Since this is stealth related, can we have a separated utils file or even a Utils class that can deal with those methods?

@jhtitor
Copy link
Contributor Author

jhtitor commented Jun 22, 2018

Are you asking if there is any specific reason for this to be part of BitShares class and not in a separate class? No, there's no specific reason for that, I just had no idea where to put it. I'll gladly move it out, however, I've also noticed that latest develop has Fee class. Should this patch be rewritten to utilize that?

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.

3 participants