-
Notifications
You must be signed in to change notification settings - Fork 123
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
Dev simple spend #129
Dev simple spend #129
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the PR @frier17. Looking good. One minor comment regarding defaulting to low fees.
blockcypher/api.py
Outdated
@@ -1624,7 +1624,7 @@ def broadcast_signed_transaction(unsigned_tx, signatures, pubkeys, coin_symbol=' | |||
|
|||
|
|||
def simple_spend(from_privkey, to_address, to_satoshis, change_address=None, | |||
privkey_is_compressed=True, min_confirmations=0, api_key=None, coin_symbol='btc'): | |||
privkey_is_compressed=True, min_confirmations=0, api_key=None, coin_symbol='btc', preference: str = 'low'): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For a better UX I suggest that keep the fee high
here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your feedback @quentinlesceller. I plan making the needed changes. Also noticed a similar need for the function simple_spend_p2sh
. Will attempt to make similar changes and add tests as well.
Thanks
Oh just noticed the zero fee. Transaction with zero fees will be directly rejected by most transaction pools so I would remove the option. |
Thanks for the feedback @quentinlesceller. I have removed the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Amazing. Thank you for the PR! LGTM!
Hi. I am proposing these changes to simple_spend with respect to issue #122 by @haysquareA. I have modified the simple_spend to take a
preference
parameter which can be used as a flag to set the transaction fee as low, medium, zero, or high in creating an unsigned transaction.I also added update to the simple_spend documentation to reflect the
preference
variable.Lastly, a test was added as SimpleSpendTX in the
test_blockcypher.py
script. This tests uses example from the blockcypher documentation with values set to fail due to insufficient funds in the address used.Kindly review.
Thanks.