Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Supported choosing asset_id to fee for simple transfer #56

Merged
merged 2 commits into from
Mar 20, 2018

Conversation

brickgao
Copy link
Contributor

#22

@xeroc
Copy link
Member

xeroc commented Mar 14, 2018

Thanks for the PR. I would like to make a request that is pretty easy to implement and enables all calls to use a different fee asset.

  1. use fee_asset to be used in **kwargs of all methods (fee_asset should be of type bitshares.asset.Asset)
  2. do not touch individual methods (like transfer) but instead modify finalizeOp (similar to append_to)
  3. Modify transactionbuilder.py and create a new method set_fee_asset that sets the fee for the object
  4. Modify constructTx and addRequeredFees to allow using a specified asset id.

That way, all calls can make use of this feature without even touching them.

@brickgao
Copy link
Contributor Author

Hi @xeroc, thanks for your advice, and I have updated.

@codecov-io
Copy link

codecov-io commented Mar 14, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@c7f9899). Click here to learn what that means.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #56   +/-   ##
=========================================
  Coverage          ?   68.15%           
=========================================
  Files             ?       50           
  Lines             ?     3925           
  Branches          ?        0           
=========================================
  Hits              ?     2675           
  Misses            ?     1250           
  Partials          ?        0
Impacted Files Coverage Δ
bitshares/bitshares.py 47.34% <50%> (ø)
bitshares/transactionbuilder.py 66.52% <75%> (ø)

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 c7f9899...fb40617. Read the comment docs.

@xeroc xeroc changed the base branch from master to develop March 20, 2018 06:02
@xeroc xeroc merged commit 5ca5ee1 into bitshares:develop Mar 20, 2018
@xeroc
Copy link
Member

xeroc commented Mar 20, 2018

Excellent work! Thank you for your contribution!

xeroc added a commit that referenced this pull request Mar 23, 2018
Release 0.1.13

77b651b (HEAD -> master) Merge branch 'release/0.1.13'
5346039 (release/0.1.13) version bump
458c293 remove ropeproject files
425eaa2 (develop) bump dependency on graphenelib
7687bec (origin/develop) [account] improve history generator
f79a3a4 [docs] improve index.rst
c5fcacb fix wrong derivation of collateral
806af6d Properly set heritance
0d72150 [chache] fix clearing of cache
4418a6a Properly load new wallet class
6fd5d59 merge improvements from python-peerplays
f6d22eb Catch account_create exception
138ba18 fix #59
5ca5ee1 Merge pull request #56 from brickgao/dev
c42d689 [bitsharesapi] allow to define list of API nodes with , or ;
fb40617 supported all calls to use a different fee asset
732d7d2 supported selecting asset_id to fee for simple transfer
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