blujay2013 / eecs588-pjct

EECS 588 Warriors, W14
0 stars 0 forks source link

Desktop: BitcoinQT recognizes transfers to multisig addresses as outgoing txns #9

Closed blujay2013 closed 10 years ago

blujay2013 commented 10 years ago

Steps to reproduce problem:

-Create a multisig script address with a pub key that you do not own in Bitcoin-QT -Transfer some BTC to the multisig address in Bitcoind -Return to Bitcoin-QT. Currently, it'll show your transaction as an outbound transaction and your wallet balance is reduced by the amount that you transferred to the multisig address.

But Bitcoind recognizes the multisig address as 'isMine':

jason-mbp:src jason$ ./bitcoin-cli validateaddress 2N5RHWjrXS8LbyatN7uqQgZReubfdLr1TfA { "isvalid" : true, "address" : "2N5RHWjrXS8LbyatN7uqQgZReubfdLr1TfA", "ismine" : true, "isscript" : true, "script" : "multisig", "hex" : "5221039d4312bf57e1ea744a5c68fb7549eedccaa7bd886a64442f5a3c58e436895ce92102c00af18bbb608fc32dcc73d4a8c15fac2009a2c681b0edd0fab5f6890f6e137152ae", "addresses" : [ "mvVed7jX15mJSnHb1kAZjKjQ7AruqkqfXq", "mvHiHZNa7HpUAPviakaZqVWPJKdfM9tA92" ], "sigsrequired" : 2, "account" : "evamobile" }

Expected behavior:

Transfers from address to multisig address within the same wallet should not result in a wallet balance deduction. Transaction should be seen as one that is owned by yourself and transfer txn should be available for redemption when user enables 2FA.

Observed behavior:

Transfer results in a wallet balance deduction. Transfer funds not available for redemption.

As a result, partially signed transactions using this signature do not work, because the wallet contains no available bit coins associated with this address:

Bitcoin-QT debug output: Script found: 3220303339643433313262663537653165613734346135633638666237353439656564636361613762643838366136343434326635613363353865343336383935636539203032633030616631386262623630386663333264636337336434613863313566616332303039613263363831623065646430666162356636383930663665313337312032204f505f434845434b4d554c5449534947 Two factor address: 2N5RHWjrXS8LbyatN7uqQgZReubfdLr1TfA Total available input blocks: 1 Current test address: 6d70573942564a6d78446f5a4c6f687256546d7342773155364e3261677035774c48 Found 2FA script: 3220303339643433313262663537653165613734346135633638666237353439656564636361613762643838366136343434326635613363353865343336383935636539203032633030616631386262623630386663333264636337336434613863313566616332303039613263363831623065646430666162356636383930663665313337312032204f505f434845434b4d554c5449534947 Number of input blocks: 1 Found number of suitable input blocks: 0 Unsigned tx hex: 0100000000022808d717ffffffff17a9145da79789876784d7f1abf61137f0f718059dad1787c8d028e8000000001976a9140fe36a332c355b87be0b73c1e0c0efd90cfcab3a88ac00000000 Serialized signed tx hex: 0100000000022808d717ffffffff17a9145da79789876784d7f1abf61137f0f718059dad1787c8d028e8000000001976a9140fe36a332c355b87be0b73c1e0c0efd90cfcab3a88ac00000000

But this is what happens in Bitcoind when you try to perform a sendrawtransaction on the above txn:

jason-mbp:src jason$ ./bitcoin-cli sendrawtransaction 0100000000022808d717ffffffff17a9145da79789876784d7f1abf61137f0f718059dad1787c8d028e8000000001976a9140fe36a332c355b87be0b73c1e0c0efd90cfcab3a88ac00000000 error: {"code":-22,"message":"TX rejected"}

And the following message appears in the debug log: 2014-04-17 13:54:56 ERROR: CheckTransaction() : vin empty 2014-04-17 13:54:56 ERROR: AcceptToMemoryPool: : CheckTransaction failed

blujay2013 commented 10 years ago

jason-mbp:src jason$ ./bitcoin-cli validateaddress 2N5RHWjrXS8LbyatN7uqQgZReubfdLr1TfA { "isvalid" : true, "address" : "2N5RHWjrXS8LbyatN7uqQgZReubfdLr1TfA", "ismine" : true, "isscript" : true, "script" : "multisig", ... "account" : "evamobile" }

jason-mbp:src jason$ ./bitcoin-cli listaccounts { "" : 38.87939000, "evamobile" : 0.00000000 }

blujay2013 commented 10 years ago

What's strange is that this transaction is currently recognized on BlockExplorer, but yet evamobile - 2N5RHWjrXS8LbyatN7uqQgZReubfdLr1TfA - still has no funds. It's as if the BTC went into limbo.

http://blockexplorer.com/testnet/tx/09151eaeebb38ab7862e9f026204eb30ac0c917b956b0831a1ed2d7b2b9b26fd

evamobile should have at least 0.05 BTC. (Of course, block explorer labels this as an unknown)

blujay2013 commented 10 years ago

OTOH I'm starting to suspect Bitcoin Core may have a bug with multisig (2FA) addresses:

jason-mbp:src jason$ ./bitcoin-cli getreceivedbyaccount evamobile 0.12001000

while

jason-mbp:src jason$ ./bitcoin-cli listaccounts { "" : 38.87939000, "evamobile" : 0.00000000 <--- !!!
}

blujay2013 commented 10 years ago

Theory:

The wallet seems to assume that payments originate from the same account - from wallet.h:

std::string strFromAccount;

and

Value listaccounts(const Array& params, bool fHelp)
{
    if (fHelp || params.size() > 1)
        throw runtime_error(
            "listaccounts ( minconf )\n"
            "\nReturns Object that has account names as keys, account balances as values.\n"
            "\nArguments:\n"
            "1. minconf     (numeric, optional, default=1) Only onclude transactions with at least this many confirmations\n"
            "\nResult:\n"
            "{                      (json object where keys are account names, and values are numeric balances\n"
            "  \"account\": x.xxx,  (numeric) The property name is the account name, and the value is the total balance for the account.\n"
            "  ...\n"
            "}\n"
            "\nExamples:\n"
            "\nList account balances where there at least 1 confirmation\n"
            + HelpExampleCli("listaccounts", "") +
            "\nList account balances including zero confirmation transactions\n"
            + HelpExampleCli("listaccounts", "0") +
            "\nList account balances for 6 or more confirmations\n"
            + HelpExampleCli("listaccounts", "6") +
            "\nAs json rpc call\n"
            + HelpExampleRpc("listaccounts", "6")
        );

    int nMinDepth = 1;
    if (params.size() > 0)
        nMinDepth = params[0].get_int();

    map<string, int64_t> mapAccountBalances;
    BOOST_FOREACH(const PAIRTYPE(CTxDestination, CAddressBookData)& entry, pwalletMain->mapAddressBook) {
        if (IsMine(*pwalletMain, entry.first)) // This address belongs to me
            mapAccountBalances[entry.second.name] = 0;
    }

    for (map<uint256, CWalletTx>::iterator it = pwalletMain->mapWallet.begin(); it != pwalletMain->mapWallet.end(); ++it)
    {
        const CWalletTx& wtx = (*it).second;
        int64_t nFee;
        string strSentAccount;
        list<pair<CTxDestination, int64_t> > listReceived;
        list<pair<CTxDestination, int64_t> > listSent;
        int nDepth = wtx.GetDepthInMainChain();
        if (wtx.GetBlocksToMaturity() > 0 || nDepth < 0)
            continue;
        wtx.GetAmounts(listReceived, listSent, nFee, strSentAccount);
void CWalletTx::GetAmounts(list<pair<CTxDestination, int64_t> >& listReceived,
                           list<pair<CTxDestination, int64_t> >& listSent, int64_t& nFee, string& strSentAccount) const
{
    nFee = 0;
    listReceived.clear();
    listSent.clear();
    strSentAccount = strFromAccount; // this is a problem 

This explains why list accounts shows evamobile as having 0 BTC, but getreceivedbyaccount is fine:

Value getreceivedbyaccount(const Array& params, bool fHelp)
{
    if (fHelp || params.size() < 1 || params.size() > 2)
        throw runtime_error(
            "getreceivedbyaccount \"account\" ( minconf )\n"
            "\nReturns the total amount received by addresses with <account> in transactions with at least [minconf] confirmations.\n"
            "\nArguments:\n"
            "1. \"account\"      (string, required) The selected account, may be the default account using \"\".\n"
            "2. minconf          (numeric, optional, default=1) Only include transactions confirmed at least this many times.\n"
            "\nResult:\n"
            "amount              (numeric) The total amount in btc received for this account.\n"
            "\nExamples:\n"
            "\nAmount received by the default account with at least 1 confirmation\n"
            + HelpExampleCli("getreceivedbyaccount", "\"\"") +
            "\nAmount received at the tabby account including unconfirmed amounts with zero confirmations\n"
            + HelpExampleCli("getreceivedbyaccount", "\"tabby\" 0") +
            "\nThe amount with at least 6 confirmation, very safe\n"
            + HelpExampleCli("getreceivedbyaccount", "\"tabby\" 6") +
            "\nAs a json rpc call\n"
            + HelpExampleRpc("getreceivedbyaccount", "\"tabby\", 6")
        );

    // Minimum confirmations
    int nMinDepth = 1;
    if (params.size() > 1)
        nMinDepth = params[1].get_int();

    // Get the set of pub keys assigned to account
    string strAccount = AccountFromValue(params[0]);
    set<CTxDestination> setAddress = pwalletMain->GetAccountAddresses(strAccount);

And this successfully shows that evamobile has 0.12001000 BTC; it is on a different account than the primary one but still owned by the same wallet.

This may be a problem ... unfortunately I need to study for crypto and please the crypto gods.

blujay2013 commented 10 years ago

Shipped with https://github.com/blujay2013/eecs588-pjct/commit/202e948dacac205685dc9435cd67c3762cc62da8