-
Notifications
You must be signed in to change notification settings - Fork 3
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
Refactor method names and where asset, balance, and import logic lives #41
Changes from 9 commits
eb36317
f89912e
7cc33ef
841a4d0
4185ff0
0f94795
ff69c26
8fed7b4
7634f72
46632bd
5e198ce
b5eaffc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -35,45 +35,32 @@ def wallet_id | |
|
||
# Returns the Address ID. | ||
# @return [String] The Address ID | ||
def address_id | ||
def id | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. should we do the same for transfer_id? |
||
@model.address_id | ||
end | ||
|
||
# Returns the balances of the Address. | ||
# @return [BalanceMap] The balances of the Address, keyed by asset ID. Ether balances are denominated | ||
# in ETH. | ||
def list_balances | ||
def balances | ||
response = Coinbase.call_api do | ||
addresses_api.list_address_balances(wallet_id, address_id) | ||
addresses_api.list_address_balances(wallet_id, id) | ||
end | ||
|
||
Coinbase.to_balance_map(response) | ||
Coinbase::BalanceMap.from_balances(response.data) | ||
end | ||
|
||
# Returns the balance of the provided Asset. | ||
# @param asset_id [Symbol] The Asset to retrieve the balance for | ||
# @return [BigDecimal] The balance of the Asset | ||
def get_balance(asset_id) | ||
normalized_asset_id = normalize_asset_id(asset_id) | ||
|
||
def balance(asset_id) | ||
response = Coinbase.call_api do | ||
addresses_api.get_address_balance(wallet_id, address_id, normalized_asset_id.to_s) | ||
addresses_api.get_address_balance(wallet_id, id, Coinbase::Asset.primary_denomination(asset_id).to_s) | ||
end | ||
|
||
return BigDecimal('0') if response.nil? | ||
|
||
amount = BigDecimal(response.amount) | ||
|
||
case asset_id | ||
when :eth | ||
amount / BigDecimal(Coinbase::WEI_PER_ETHER.to_s) | ||
when :gwei | ||
amount / BigDecimal(Coinbase::GWEI_PER_ETHER.to_s) | ||
when :usdc | ||
amount / BigDecimal(Coinbase::ATOMIC_UNITS_PER_USDC.to_s) | ||
Comment on lines
-68
to
-73
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was not handling |
||
else | ||
amount | ||
end | ||
Coinbase::Balance.from_model_and_asset_id(response, asset_id).amount | ||
end | ||
|
||
# Transfers the given amount of the given Asset to the given address. Only same-Network Transfers are supported. | ||
|
@@ -83,36 +70,32 @@ def get_balance(asset_id) | |
# default address. If a String, interprets it as the address ID. | ||
# @return [String] The hash of the Transfer transaction. | ||
def transfer(amount, asset_id, destination) | ||
raise ArgumentError, "Unsupported asset: #{asset_id}" unless Coinbase::SUPPORTED_ASSET_IDS[asset_id] | ||
raise ArgumentError, "Unsupported asset: #{asset_id}" unless Coinbase::Asset.supported?(asset_id) | ||
|
||
if destination.is_a?(Wallet) | ||
raise ArgumentError, 'Transfer must be on the same Network' if destination.network_id != network_id | ||
|
||
destination = destination.default_address.address_id | ||
destination = destination.default_address.id | ||
elsif destination.is_a?(Address) | ||
raise ArgumentError, 'Transfer must be on the same Network' if destination.network_id != network_id | ||
|
||
destination = destination.address_id | ||
destination = destination.id | ||
end | ||
|
||
current_balance = get_balance(asset_id) | ||
current_balance = balance(asset_id) | ||
if current_balance < amount | ||
raise ArgumentError, "Insufficient funds: #{amount} requested, but only #{current_balance} available" | ||
end | ||
|
||
normalized_amount = normalize_asset_amount(amount, asset_id) | ||
|
||
normalized_asset_id = normalize_asset_id(asset_id) | ||
|
||
create_transfer_request = { | ||
amount: normalized_amount.to_i.to_s, | ||
amount: Coinbase::Asset.to_atomic_amount(amount, asset_id).to_i.to_s, | ||
network_id: network_id, | ||
asset_id: normalized_asset_id.to_s, | ||
asset_id: Coinbase::Asset.primary_denomination(asset_id).to_s, | ||
destination: destination | ||
} | ||
|
||
transfer_model = Coinbase.call_api do | ||
transfers_api.create_transfer(wallet_id, address_id, create_transfer_request) | ||
transfers_api.create_transfer(wallet_id, id, create_transfer_request) | ||
end | ||
|
||
transfer = Coinbase::Transfer.new(transfer_model) | ||
|
@@ -127,7 +110,7 @@ def transfer(amount, asset_id, destination) | |
} | ||
|
||
transfer_model = Coinbase.call_api do | ||
transfers_api.broadcast_transfer(wallet_id, address_id, transfer.transfer_id, broadcast_transfer_request) | ||
transfers_api.broadcast_transfer(wallet_id, id, transfer.transfer_id, broadcast_transfer_request) | ||
end | ||
|
||
Coinbase::Transfer.new(transfer_model) | ||
|
@@ -136,7 +119,7 @@ def transfer(amount, asset_id, destination) | |
# Returns a String representation of the Address. | ||
# @return [String] a String representation of the Address | ||
def to_s | ||
"Coinbase::Address{address_id: '#{address_id}', network_id: '#{network_id}', wallet_id: '#{wallet_id}'}" | ||
"Coinbase::Address{id: '#{id}', network_id: '#{network_id}', wallet_id: '#{wallet_id}'}" | ||
end | ||
|
||
# Same as to_s. | ||
|
@@ -148,11 +131,11 @@ def inspect | |
# Requests funds for the address from the faucet and returns the faucet transaction. | ||
# This is only supported on testnet networks. | ||
# @return [Coinbase::FaucetTransaction] The successful faucet transaction | ||
# @raise [Coinbase::FaucetLimitReached] If the faucet limit has been reached for the address or user. | ||
# @raise [Coinbase::FaucetLimitReachedError] If the faucet limit has been reached for the address or user. | ||
# @raise [Coinbase::Client::ApiError] If an unexpected error occurs while requesting faucet funds. | ||
def faucet | ||
Coinbase.call_api do | ||
Coinbase::FaucetTransaction.new(addresses_api.request_faucet_funds(wallet_id, address_id)) | ||
Coinbase::FaucetTransaction.new(addresses_api.request_faucet_funds(wallet_id, id)) | ||
end | ||
end | ||
|
||
|
@@ -162,61 +145,30 @@ def export | |
@key.private_hex | ||
end | ||
|
||
# Lists the IDs of all Transfers associated with the given Wallet and Address. | ||
# @return [Array<String>] The IDs of all Transfers belonging to the Wallet and Address | ||
def list_transfer_ids | ||
transfer_ids = [] | ||
# Returns all of the transfers associated with the address. | ||
# @return [Array<Coinbase::Transfer>] The transfers associated with the address | ||
def transfers | ||
transfers = [] | ||
page = nil | ||
|
||
loop do | ||
puts "fetch transfers page: #{page}" | ||
response = Coinbase.call_api do | ||
transfers_api.list_transfers(wallet_id, address_id, { limit: 100, page: page }) | ||
transfers_api.list_transfers(wallet_id, id, { limit: 100, page: page }) | ||
end | ||
|
||
transfer_ids.concat(response.data.map(&:transfer_id)) if response.data | ||
transfers.concat(response.data.map { |transfer| Coinbase::Transfer.new(transfer) }) if response.data | ||
|
||
break unless response.has_more | ||
|
||
page = response.next_page | ||
end | ||
|
||
transfer_ids | ||
transfers | ||
end | ||
|
||
private | ||
|
||
# Normalizes the amount of the Asset to send to the atomic unit. | ||
# @param amount [Integer, Float, BigDecimal] The amount to normalize | ||
# @param asset_id [Symbol] The ID of the Asset being transferred | ||
# @return [BigDecimal] The normalized amount in atomic units | ||
def normalize_asset_amount(amount, asset_id) | ||
big_amount = BigDecimal(amount.to_s) | ||
|
||
case asset_id | ||
when :eth | ||
big_amount * Coinbase::WEI_PER_ETHER | ||
when :gwei | ||
big_amount * Coinbase::WEI_PER_GWEI | ||
when :usdc | ||
big_amount * Coinbase::ATOMIC_UNITS_PER_USDC | ||
when :weth | ||
big_amount * Coinbase::WEI_PER_ETHER | ||
else | ||
big_amount | ||
end | ||
end | ||
|
||
# Normalizes the asset ID to use during requests. | ||
# @param asset_id [Symbol] The asset ID to normalize | ||
# @return [Symbol] The normalized asset ID | ||
def normalize_asset_id(asset_id) | ||
if %i[wei gwei].include?(asset_id) | ||
:eth | ||
else | ||
asset_id | ||
end | ||
end | ||
|
||
def addresses_api | ||
@addresses_api ||= Coinbase::Client::AddressesApi.new(Coinbase.configuration.api_client) | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
# frozen_string_literal: true | ||
|
||
module Coinbase | ||
# A representation of an Balance. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What's the difference in your opinion there? We can clarify it is an amount of a specific asset? |
||
class Balance | ||
# Converts a Coinbase::Client::Balance model to a Coinbase::Balance | ||
# @param balance_model [Coinbase::Client::Balance] The balance fetched from the API. | ||
# @return [Balance] The converted Balance object. | ||
def self.from_model(balance_model) | ||
asset_id = Coinbase.to_sym(balance_model.asset.asset_id.downcase) | ||
|
||
from_model_and_asset_id(balance_model, asset_id) | ||
end | ||
|
||
# Converts a Coinbase::Client::Balance model and asset ID to a Coinbase::Balance | ||
# This can be used to specify a non-primary denomination that we want the balance | ||
# to be converted to. | ||
# @param balance_model [Coinbase::Client::Balance] The balance fetched from the API. | ||
# @param asset_id [Symbol] The Asset ID of the denomination we want returned. | ||
# @return [Balance] The converted Balance object. | ||
def self.from_model_and_asset_id(balance_model, asset_id) | ||
new( | ||
amount: Coinbase::Asset.from_atomic_amount(BigDecimal(balance_model.amount), asset_id), | ||
asset_id: asset_id | ||
) | ||
end | ||
|
||
# Returns a new Asset object. Do not use this method. Instead, use the Asset constants defined in | ||
# the Coinbase module. | ||
# @param network_id [Symbol] The ID of the Network to which the Asset belongs | ||
# @param asset_id [Symbol] The Asset ID | ||
# @param display_name [String] The Asset's display name | ||
# @param address_id [String] (Optional) The Asset's address ID, if one exists | ||
def initialize(amount:, asset_id:) | ||
@amount = amount | ||
@asset_id = asset_id | ||
end | ||
|
||
attr_reader :amount, :asset_id | ||
end | ||
end |
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.
update line 174 to
w5 = wallets[w3.id]
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.
Can we make the README / example something that is actually evaluated?