search

coinbase / coinbase-python

DEPRECATED — Coinbase Python API
Apache License 2.0
524 stars 218 forks source link

Added currency and payment_method params as required to the buy method #78

Closed sheffield-nolan closed 6 years ago

sheffield-nolan commented 6 years ago

The 'currency' param is required by the API endpoint, while the 'payment_method' param is not, but should be to avoid unintended funding from linked accounts.

Updated the unit tests to support the additional required params.

sds commented 6 years ago

Thanks for the PR, @sheffield-nolan. Can you elaborate on the defensive measure you've put in place with the payment_method argument? I'm not sure I fully understand the situation we're protecting against.

Otherwise this looks great!

sheffield-nolan commented 6 years ago

@sds - I made the payment_method required as a safeguard, as the current buy method in the API is akin to having an empty (optional) field for the funding source on the CB website for buys, with no confirmation dialog.

Here is the scenario that happened to me and would like to prevent for other developers:

  1. Developer links bank account years ago.
  2. Developer buys $10 USD of ETH using a debit card to test with, using CB website with confirmation safeguard.
  3. Developer sells $10 USD of ETH on CB website to create small fiat account to fund testing.
  4. Developer attempts to buy $1 USD of ETH to deposit into their CB ETH account using the API.
  5. 'BTC' is accidentally used instead of 'USD' as the currency param.
  6. Thousands of dollars are debited from the linked bank account, since payment_method was optional.

There is no recourse or rollback from this scenario and it could have all been prevented by having the developer explicitly indicate the payment_method.

sds commented 6 years ago

Thanks for clarifying!