Replaces core transaction, address, serialization
and sign functionality for Bitcoin with
python-bitcointx backend.
Removes bech32 and btscript
modules from jmbitcoin. Removes all string,
hex, binary conversion routines. A generic
hex/binary conversion now is added to jmbase.
Removes all transaction serialization and
deserialization routines. Removes the now
irrelevant test modules.
Remaining functions in jmbitcoin remove any parsing of
hex format, requiring callers to use binary only.
One additional test added, testing the remaining
function in secp256k1_transaction.py: the signing
of transactions. Deserialized form is now
bitcointx.CMutableTransaction.
For jmbase, in addition to the above, generic conversions
for utxos to and from strings is added, and a dynamic conversion
for AMP messages to binary-only. Within the code, utxos are
now only in (binarytxid, int) form, except where converted
for communcation.
Tthe largest part of the changes are
the modifications to jmbitcoin calls in jmclient;
as well as different encapsulation with CMutableTransaction,
there is also a removal of some but not all hex parsing;
it remains for rpc calls to Core and for AMP message
parsing. Backwards compatibility must be ensured so some
joinmarket protocol messages still use hex, and it is
also preserved in persistence of PoDLE data.
As part of this, some significant simplification of
certain legacy functions within the wallet has been done.
jmdaemon is entirely unaltered (save for one test which
simulates jmclient code).
Reasoning for this change: to ensure that Qt will show
a message and gracefully exit (or quit attempting to
load a wallet) in all 3 cases: on startup it show an
intelligible message if the RPC connection fails (as
before this PR), if the RPC fails while no wallet is
loaded and thus no wallet service is started, it should
show an intelligible error message when you attempt to
load a wallet and it fails, and finally it should show
an intelligible error message before quitting, if the rpc
connection fails during the period when the wallet is
already loaded.
By switching to an Exception instead of sys.exit, it does
mean that starting a yieldgenerator shows a stack trace,
but it also shows an intelligible error message (in red),
and this is command line, so UI requirements are less strong.
We preserve the "good" behaviour of no stack trace, but
only error message, if the rpc connection is lost during
running.
Fixes#442.
First, the CONNREFUSED socket error is handled in jsonrpc.
Second, we respond to this (but *not* to resets) with a reactor
shutdown in BitcoinCoreInterface.rpc(). This also necessitates
early-quitting in the calling function
(WalletService.transaction_monitor) since the reactor stop
will only stop future deferred calls, not the currently running
one. The obvious sys.exit approach is only used in startup,
because the reactor is not currently running at that point.
Also minor change to DummyBlockchainInterface for test.
updated xenial and stretch dockerfiles since they bundle old versions of libsodium
refacatoring to make BlockchainInterface and BitcoinCoreNoHistoryInterface python2 compatible
add the -acceptnonstdtxn flag when invoking bitcoind for versions >= 0.19.0
refactor the use of nonlocal to make it compatible with python2
document the get_bitcoind_version method
No-history is a method for synchronizing a wallet by scanning the UTXO
set. It can be useful for checking whether seed phrase backups have
money on them before committing the time and effort required to
rescanning the blockchain. No-history sync is compatible with pruning.
The sync method cannot tell which empty addresses have been used, so
cannot guarentee avoidance of address reuse. For this reason no-history
sync disables wallet address generation and can only be used with
wallet-tool and for sending transactions without change addresses.
Introduces WalletService object which is in control of
blockchain and wallet access.
The service manages a single transaction monitoring loop,
instead of multiple, and allows updates to the wallet from
external sources to be handled in real time, so that both Qt
and other apps (yg) can respond to deposits or withdrawals
automatically.
The refactoring also controls access to both wallet and
blockchain so that client apps (Taker, Maker) will not need
to be changed for future new versions e.g. client-side filtering.
Also updates and improves Wallet Tab behaviour in Qt (memory
of expansion state).
Additionally, blockchain sync is now --fast by default, with
the former default of detailed sync being renamed --recoversync.
This reverts commit 4b4f8c97f4.
Due to a flawed assumption where sometimes importing addresses does
take a significant amount of time. For example on machines with HDD
importing 1-2 addresses can last a second or two, and importing 60
addresses lasts about 30 seconds. The commit makes running a maker
impossible on such a machine.
Prior to this commit, addresses imported on tx creation
were at the tip of the index only, but detailed sync
requires all address gap-forwards to be imported, else
it requires a restart.
The particular fix approach taken her is aggression
because import is cheap; we import wallet.gaplimit forwards
on all branches, taking advantage of the bci's function
_collect_addresses_gap, thus ensuring that unless the gap
limit changes, we will definitely not need a restart.
Fast sync did not suffer from this and is unchanged.
Prior to this commit, the regtest testing framework
used the bitcoin rpc command `generate` to mine coins
in preparation for tests, and to tick forward the
chain for confirmations. This rpc command is deprecated
and will be removed in 0.19. Hence this commit changes
the two usages of `generate` to `generatetoaddress` and
specifies a mining address created with `getnewaddress`.
Prior to this commit, if the call to the bitcoin rpc
gettransaction resulted in an exception other than JsonRpcError,
the code would crash as the variable `res` was uninitialized.
This fixes that by always setting res to None in this case, and
also promotes the logging messages for such errors to warning.
If the user manually chooses a bitcoin tx fee, it could end up below the bitcoin network's min relay fee.
This PR get the bitcoin core instance's configured min relay fee and uses it as a floor value (btccore) or uses 1000 as floor otherwise (btc as fallback when RPC call returns no good value and for electrum interface).
Previous to this commit, the jmbitcoin package was accessed via
a file in jmclient `btc.py` which was originally added as an
interface to allow the client to use a non-jmbitcoin package to
provide the implementation; while this idea is useful, the way
it was implemented was not, moreover it is not currently used
and contained duplicated code that was unmanaged. Also, the
original usage of this was only by the electrum plugin, which
has currently been abandoned. This simplifies the code and
avoids spurious error messages. Note that most of the changes
are a result of pulling the logging function directly from the
jmbase package instead of indirectly via this interface (which
was unnecessary and not connected with jmbitcoin).
Previous to this commit, if a utxo string did not conform to
the standard txid:n in hex format, of the correct length, an
exception would be thrown (specifically for invalid values of
the output index, which must be parseable as an integer). This
could allow a taker to crash a maker by sending an invalid
commitment string. After this commit this invalid utxo is
correctly ignored (and the commitment of course is invalid).
Bitcoin Core 0.17 deprecates the accounts feature and replaces it with
labels. The RPC functions using accounts still work for use with older
version of Core.
This case can happen if you sweep to a destination under the control
of your Core wallet (although it's rare). This is not actually a problem,
since that address neither was nor will be Joinmarket's. Hence show a
warning but continue. Also add a comment to BitcoinCoreInterface.im-
port_addresses, to note that it must *only* be used for initial sync,
since it has a risk of shutting down the program.
This fails (sys.exit) when importaddress throws the error corresponding to
Core wallet already owning the key corresponding to an address, and gives
brief advice how the error can be fixed.
It is not considered safe to simply ignore this error in case address reuse
or coins being sent to an address the user doesn't control, occurs.
confirmation as per TIMEOUT section in config.
Mark wallet as synced after sync_unspent in Core
interface (to match Electrum interface, and wait for
this update before modifying orders).
To correspond, allow fast sync in test setup in several tests.
Disallow sync_unspent in syncing wallet full mode if sync_addresses fails.
This prevents the wallet erroneously being marked as synced in full mode.
This change happened as a result of the new electrum wallet sync; because
this is asynchronous, the call to sync_unspent in the client protocol does
not block, thus must be flagged when complete, this is done with
bci.wallet_synced set to True at end of sync_unspent. But this leads
to the wallet as always being marked synced using full sync_mode if we
call both functions unconditionally.
These wallet sync conditions have become unwieldy; a refactor is in order.
Some small extra documentation in docs/TODO.md.
handle electrum server disconnects and switch to new servers automatically, refactor server lists to data file
add SSL connections to electrum
Bugfixes in electrum code:
Restart on connectionFailed as well as connectionLost
Query utxo set must return (only) None for nonexistent/spent utxos.
Dont include twisted logging for wallet-tool
start thread immediately for early blockchain queries
timeout non-responsive servers
fix abstractBCI refs in subclasses
Kristaps Kaupe
jules23
Adam Gibson
chris-belcher
James Hilliard
Tim Akinbo
cindiv
AlexCato
undeath
Alex Cato