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

Feature/new v3 api #19

Open
wants to merge 16 commits into
base: main
Choose a base branch
from
Open

Conversation

luckfamousa
Copy link

First version of the extension for the V3 API.

@luckfamousa luckfamousa reopened this Nov 12, 2024
@qoomon
Copy link

qoomon commented Nov 16, 2024

@luckfamousa I get following error.

CoinbaseV3.lua: CoinbaseV3.lua:186: attempt to index a nil value (field 'integer index')

@luckfamousa
Copy link
Author

Are you sure that you entered your private key in the correct format?
You might add a print statement that prints the extracted key after gsub to check:

    key = string.match(key, "BEGIN EC PRIVATE KEY%-%-%-%-%-%s*(.*)%-%-%-%-%-END EC PRIVATE KEY")
    key = string.gsub(key, "%s+", "")
    print(key)
    key = MM.base64decode(key)

@qoomon
Copy link

qoomon commented Nov 16, 2024

my bad, i copied the json of the key. that included \n chars. Its working thanks

src/CoinbaseV3.lua Outdated Show resolved Hide resolved
src/CoinbaseV3.lua Outdated Show resolved Hide resolved
src/CoinbaseV3.lua Outdated Show resolved Hide resolved
@luckfamousa
Copy link
Author

Sure. Does it work for you this way?
So the old V2 API is no longer available and there will be no use case where someone will want to use both in parallel?

@qoomon
Copy link

qoomon commented Nov 16, 2024

@luckfamousa first of all I forgot to say thank you for this extension update.

Sure. Does it work for you this way?

Yes it works as expected.

So the old V2 API is no longer available and there will be no use case where someone will want to use both in parallel?

No that would not make no sense

@qoomon
Copy link

qoomon commented Nov 16, 2024

works like a charm

@qoomon
Copy link

qoomon commented Nov 16, 2024

One last idea, maybe it make sense to filter currencies with no or zero value.

@qoomon
Copy link

qoomon commented Nov 16, 2024

Awesome. I guess you missed the filter for account["type"] == "ACCOUNT_TYPE_FIAT"

@qoomon
Copy link

qoomon commented Nov 16, 2024

And for some reason for some accounts the currency is %, very strange, for most currencies it is my account currency

@qoomon
Copy link

qoomon commented Nov 16, 2024

It appears when I add () to currency for non fiat currencies (currency = account["currency"] .. "()",) it works as expected

@qoomon
Copy link

qoomon commented Nov 16, 2024

just remove currency = account["currency"] works as well

@qoomon
Copy link

qoomon commented Nov 16, 2024

This is my version
-- Inofficial Coinbase Extension (www.coinbase.com) for MoneyMoney
-- Fetches balances from Coinbase API V3 and returns them as securities
--
-- Username: Coinbase Key Name
-- Password: Coinbase Private Key
--
-- Copyright (c) 2024 Felix Nensa
-- Copyright (c) 2020-2022 Martin Wilhelmi
-- Copyright (c) 2017 Nico Lindemann
--
-- Permission is hereby granted, free of charge, to any person obtaining a copy
-- of this software and associated documentation files (the "Software"), to deal
-- in the Software without restriction, including without limitation the rights
-- to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
-- copies of the Software, and to permit persons to whom the Software is
-- furnished to do so, subject to the following conditions:
--
-- The above copyright notice and this permission notice shall be included in all
-- copies or substantial portions of the Software.
--
-- THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
-- IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
-- FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
-- AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
-- LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
-- OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
-- SOFTWARE.

WebBanking {
    version = 3.0,
    url = "https://api.coinbase.com",
    description = "Fetch balances from Coinbase API V3 and list them as securities",
    services = { "Coinbase Account" }
}

local apiKey
local apiSecret
local currency
local balances
local prices
local api_host = "api.coinbase.com"
local api_path = "/api/v3/brokerage/"
local market = "Coinbase"
local accountNumber = "Main"

function SupportsBank (protocol, bankCode)
    return protocol == ProtocolWebBanking and bankCode == "Coinbase Account"
end

function InitializeSession (protocol, bankCode, username, username2, password, username3)
    apiKey = username
    apiSecret = password
    -- Take currency of first fiat account, ignores pagination limit (default: 49)
    -- See: https://docs.cdp.coinbase.com/advanced-trade/reference/retailbrokerageapi_getaccounts/
    local accounts = queryPrivate("accounts")
    local size = accounts["size"]
    for i = 1, size do
        local account = accounts["accounts"][i]
        if account["type"] == "ACCOUNT_TYPE_FIAT" then
            currency = account["currency"]
            break
        end
    end
end

function ListAccounts (knownAccounts_notused)
    local account = {
        name = market,
        accountNumber = accountNumber,
        currency = currency,
        portfolio = true,
        type = "AccountTypePortfolio"
    }
    return {account}
end

function RefreshAccount(account_notused, since_notused)
    local s = {}
    local accounts = queryPrivate("accounts")

    local size = accounts["size"]
    for i = 1, size do
        local account = accounts["accounts"][i]
        if account["type"] == "ACCOUNT_TYPE_FIAT" and tonumber(account["available_balance"]["value"]) > 0 then
            s[#s+1] = {
                name = account["name"],
                market = market,
                currency = account["currency"],
                amount = tonumber(account["available_balance"]["value"])
            }
        else
            local prices = queryPrivate("market/products/" .. account["currency"] .. "-" .. currency)

            if prices ~= nil and prices["error"] == nil and tonumber(account["available_balance"]["value"]) > 0 then
                s[#s+1] = {
                    name = account["name"],
                    market = market,
                    -- currency = account["currency"],
                    quantity = tonumber(account["available_balance"]["value"]),
                    amount = tonumber(account["available_balance"]["value"]) * tonumber(prices["price"]),
                    price = tonumber(prices["price"])
                }
            end
        end
    end

    return {securities = s}
end

function EndSession ()
end

-------------------- Helper functions --------------------

-- Function to pad a byte string to a specific length with leading zeros
local function pad_to_length(data, length)
    if #data < length then
        data = string.rep("\0", length - #data) .. data
    elseif #data > length then
        data = data:sub(-length)  -- Trim to the required length if longer
    end
    return data
end

-- Function to convert a DER-encoded signature to concatenated r || s format
-- Based on https://stackoverflow.com/a/69109085/5347900
local function der_to_concat_rs(der_signature)
    local pos = 1
    assert(der_signature:byte(pos) == 0x30, "Expected SEQUENCE")
    pos = pos + 1  -- Skip SEQUENCE tag

    local length = der_signature:byte(pos)
    pos = pos + 1

    -- Extract r value
    assert(der_signature:byte(pos) == 0x02, "Expected INTEGER for r")
    pos = pos + 1
    local r_len = der_signature:byte(pos)
    pos = pos + 1
    local r = der_signature:sub(pos, pos + r_len - 1)
    pos = pos + r_len

    -- Extract s value
    assert(der_signature:byte(pos) == 0x02, "Expected INTEGER for s")
    pos = pos + 1
    local s_len = der_signature:byte(pos)
    pos = pos + 1
    local s = der_signature:sub(pos, pos + s_len - 1)
    pos = pos + s_len

    -- Ensure r and s are 32 bytes by adding leading zeros if necessary
    r = pad_to_length(r, 32)
    s = pad_to_length(s, 32)

    -- Concatenate r || s
    return r .. s
end

-- Function to create JWT token using ES256 and MM helper functions
function create_jwt(apiSecret, header, payload)
    local json_header = JSON():set(header):json()
    local json_payload = JSON():set(payload):json()

    -- Ensure header and payload are BASE64 encoded
    local encoded_header = MM.base64urlencode(json_header)
    local encoded_payload = MM.base64urlencode(json_payload)

    local signature_input = encoded_header .. "." .. encoded_payload

    -- Load and parse the EC private key
    local key = apiSecret
    key = string.match(key, "BEGIN EC PRIVATE KEY%-%-%-%-%-%s*(.*)%-%-%-%-%-END EC PRIVATE KEY")
    key = string.gsub(key, "%s+", "")
    key = MM.base64decode(key)

    local der = MM.derdecode(MM.derdecode(key)[1][2])
    local d = der[2][2]
    local p = MM.derdecode(der[4][2])[1][2]
    local x = string.sub(p, string.len(p) - 63, string.len(p) - 32)
    local y = string.sub(p, string.len(p) - 31)

    -- Sign the data using MM.ecSign
    local signature = MM.ecSign({curve="prime256v1", d=d, x=x, y=y}, signature_input, "ecdsa sha256")
    local rs_signature = der_to_concat_rs(signature)
    local encoded_signature = MM.base64urlencode(rs_signature)

    -- Construct and return the JWT
    return encoded_header .. "." .. encoded_payload .. "." .. encoded_signature
end

-- Generate a hexadecimal nonce
local function generate_hex_nonce(length)
    local res = {}
    local hex_chars = '0123456789abcdef'

    for i = 1, length do
        local rand_index = math.random(1, #hex_chars)
        table.insert(res, hex_chars:sub(rand_index, rand_index))
    end

    return table.concat(res)
end

-- Query the advanced trade API with a private method
-- See: https://docs.cdp.coinbase.com/advanced-trade/reference/
function queryPrivate(method)
    local request_method = "GET"
    local nonce = generate_hex_nonce(64)
    local uri = request_method .. " " .. api_host .. api_path .. method
    local nbf = os.time()
    local exp = nbf + 120
    local header = {alg = "ES256", kid = apiKey, nonce = nonce, typ = "JWT"}
    local payload = {sub = apiKey, iss = "cdp", nbf = nbf, exp = exp, uri = uri}
    local jwt_token = create_jwt(apiSecret, header, payload)

    local path = api_path .. method
    local headers = {}
    headers["Accept"] = "application/json"
    headers["Authorization"] = "Bearer " .. jwt_token

    local connection = Connection()
    local content = connection:request("GET", url .. path, nil, nil, headers)

    local json = JSON(content)
    return json:dictionary()
end

@luckfamousa
Copy link
Author

tonumber(account["available_balance"]["value"]) > 0

I am not sure about this one. Are you sure you cannot have a negative balance? At least on a fiat account?

@qoomon
Copy link

qoomon commented Nov 16, 2024

Good Point, negative values should not be filtered.

@mnin
Copy link
Owner

mnin commented Nov 17, 2024

Thanks, tested it, required("math") isn't working for me. And also MM isn't available. Are you missed extra lua files?

@luckfamousa
Copy link
Author

required("math") isn't working for me.

I have pushed a version that doesn't use "math" anymore.

And also MM isn't available. Are you missed extra lua files?

The MM gets injected via the MoneyMoney sandbox and provides access to some MoneyMoney specific functions. If that is not available you seem to have an issue with your setup. What are the outputs in your MoneyMoney protocol window when the extension gets loaded?

@qoomon
Copy link

qoomon commented Nov 17, 2024

LGTM

@mnin
Copy link
Owner

mnin commented Nov 17, 2024

I reinstalled latest MoneyMoney version (2.4.44 (466) Beta) via homebrew. Just installed the latest beta and disabled "Verify digital signatures of extensions" of course.

Error is:

          Coinbase.lua: Coinbase.lua:185: attempt to index a nil value (field 'integer index')

seems to be line:

  local der = MM.derdecode(MM.derdecode(key)[1][2])

So may the undocumented functions are not available.

@luckfamousa
Copy link
Author

Error is:

          Coinbase.lua: Coinbase.lua:185: attempt to index a nil value (field 'integer index')

This means that you messed up your private key a bit when entering it into MoneyMoney.
Make sure that you enter it exactly like this (including linebreaks!!):

-----BEGIN EC PRIVATE KEY-----
M ... 9
A ... M
m ... ==
-----END EC PRIVATE KEY-----

@mnin
Copy link
Owner

mnin commented Nov 17, 2024

Just copied (keys deleted from Coinbase already):

api key: organizations/24f4c1ec-ebbd-46b3-84b9-379e559b9246/apiKeys/0226fe2e-057b-49e8-9c93-f331ae21ce07
api secret: -----BEGIN EC PRIVATE KEY-----\nMHcCAQEEID6FhvZUjcuAIwbx3n9wBQWiuwtOB9x1aOyZSoi742f6oAoGCCqGSM49\nAwEHoUQDQgAE239jJaUh1dqjofGMAXPaQJOTIob/ii6VnetwY13oKDltd/VLxdNa\n+z5P1ykYIvXQ2eaVo5KvsB4WRshSC71m+w==\n-----END EC PRIVATE KEY-----\n

Tried of course also removing the newline character at like -----BEGIN EC PRIVATE KEY-----\nMHcCAQEEID6FhvZUjcuAIwbx3n9wBQWiuwtOB9x1aOyZSoi742f6oAoGCCqGSM49\nAwEHoUQDQgAE239jJaUh1dqjofGMAXPaQJOTIob/ii6VnetwY13oKDltd/VLxdNa\n+z5P1ykYIvXQ2eaVo5KvsB4WRshSC71m+w==\n-----END EC PRIVATE KEY-----

@qoomon
Copy link

qoomon commented Nov 17, 2024

@mnin you need to remove the \n chars, I did the same mistake.

@luckfamousa
Copy link
Author

@mnin you need to remove the \n chars, I did the same mistake.

Replace \n with real linebreaks.

@qoomon
Copy link

qoomon commented Nov 17, 2024

@luckfamousa maybe it's possible to automatically do strip those for people like me that existential copy the json encoded multiline key.

@mnin
Copy link
Owner

mnin commented Nov 17, 2024

Cool, so put into the input field just multiline string.

Now getting the next error:

Coinbase.lua: Coinbase.lua:94: attempt to concatenate a nil value (upvalue 'currency')

@luckfamousa
Copy link
Author

@luckfamousa maybe it's possible to automatically do strip those for people like me that existential copy the json encoded multiline key.

Exactly what I just thought.

@mnin
Copy link
Owner

mnin commented Nov 17, 2024

First account's in the array are following types:

          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO

So currency is still nil.

@luckfamousa
Copy link
Author

First account's in the array are following types:

          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO

So currency is still nil.

Do you not have at least one ACCOUNT_TYPE_FIAT account?

@qoomon
Copy link

qoomon commented Nov 17, 2024

key = string.gsub(key, "\\n", "")

@mnin
Copy link
Owner

mnin commented Nov 17, 2024

This is the complete list:

          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_VAULT
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_VAULT
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO
          ACCOUNT_TYPE_CRYPTO

So, nope :-)

@luckfamousa
Copy link
Author

Now it defaults to 'EUR' if no fiat account is present.

@mnin
Copy link
Owner

mnin commented Nov 17, 2024

Okay cool, now the script is running fine. But it's not rendering any data.

Seeing some INVALID_ARGUMENT errors in the console:

22:00:26  1> Received: {"error":"INVALID_ARGUMENT","error_details":"valid product_id is required","message":"valid product_id is required"}
          1> Sending: GET https://api.coinbase.com/api/v3/brokerage/market/products/AMP-EUR

@luckfamousa
Copy link
Author

Don't you have any accounts that are convertible to EUR via Coinbase? BTC for instance?

22:00:26  1> Received: {"error":"INVALID_ARGUMENT","error_details":"valid product_id is required","message":"valid product_id is required"}
          1> Sending: GET https://api.coinbase.com/api/v3/brokerage/market/products/AMP-EUR

@qoomon
Copy link

qoomon commented Nov 18, 2024

For me it is working just fine

@mnin
Copy link
Owner

mnin commented Nov 20, 2024

Don't you have any accounts that are convertible to EUR via Coinbase? BTC for instance?

22:00:26  1> Received: {"error":"INVALID_ARGUMENT","error_details":"valid product_id is required","message":"valid product_id is required"}
          1> Sending: GET https://api.coinbase.com/api/v3/brokerage/market/products/AMP-EUR

I have several accounts there for many currencies. But my account is a bit old.

@luckfamousa
Copy link
Author

I have several accounts there for many currencies. But my account is a bit old.

Assuming you have a BTC account that is not empty or another account that is convertible to EUR, it should appear in your list. If it is not convertible to EUR, https://api.coinbase.com/api/v3/brokerage/market/products/XXX-EUR will fail and it will not appear in your list.

@qoomon
Copy link

qoomon commented Nov 22, 2024

@mnin will you make/request for a signed release?

@mnin
Copy link
Owner

mnin commented Nov 22, 2024

@qoomon for some new stuff which won't work for half of the users? Still working on my solution.

@mnin
Copy link
Owner

mnin commented Dec 19, 2024

Now it seems the Coinbase API is broken somehow. And the support simply doesn't respond.

@luckfamousa
Copy link
Author

Coinbase API is broken somehow

The V3 is working without any problems for me. I can watch myself losing money right now 😭🤷‍♂️

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