Prior to this commit, several test functions were using
"True" to flag internal and "False" to flag external for
the HD branch for the wallet, but we now use BaseWallet vars
ADDRESS_TYPE_[IN/EX]TERNAL (1/0), so this
is changed to explicitly reference those. There is no change
to the live code (which calls get_[internal/external]_addr).
In addition _index_cache updates in wallet are protected
with a wrapper function to ensure that the branch requested
is valid.
Tests pass both before and after this change.
See #772. This is likely a partial fix but that issue
may be more complex.
More generally, while we may have to process multiple
entries in the return of `listtransactions`, with the
same txid, because they may have different wallet labels,
we do not want to call `gettransaction` repeatedly on
the same txid in the same monitor loop call. Note however,
that we *do* need to call `gettransaction` again in
the next monitor loop, since the state (confirmations)
updates, so we cannot permanently cache those results.
Additionally removed redundant old_txs entries with set().
Prior to this commit, the list of used addresses,
which is required to check for address reuse, is populated
on startup in fast sync, and updated as new transactions
arrive; but if --recoversync is chosen, this list was not
originally getting populated. This commit corrects that bug.
Prior to this commit, in case an RPC failure occurred when
accesing the block height, the program would continue but the
wallet would be in an un-writeable state (for command line
programs, specifically yield generators; for Qt the shutdown
would occur).
This commit slightly cleans up the process of shutting down,
ensuring that duplicate shutdown calls do not result in
stack traces. It also ensures that also for command line
programs, the application will immediately shutdown if the
regular heartbeat call to query the block height fails, as this
risks inconsistencies in the wallet (though the previous
situation luckily did not result in this as the call to
BaseWallet.close() resulted in the wallet being read only).
A future PR should develop a more sophisticated approach to
RPC call failures that may allow the program to wait.
stopservice
Prior to this commit, if non-self broadcast was enabled
but the counterparty chosen did not broadcast, the transaction
would remain unbroadcast.
After this commit, the Taker checks, after the configured
value of TIMEOUT.unconfirm_timeout_sec (default 90s), the
Taker will broadcast the transaction.
Also amended default config comment for this function.
Update no-history-sync code:
This updates the new functionality in jmclient.wallet_utils
in the no-history-sync PR #444 to be compatible
with the python-bitcointx refactoring.
Remove all future/py2 compatibility code remaining:
This is in line with #525 and corrects erroneous
addition of more compatibility code.
Addresses all flake8 complaints (ununsed imports etc)
Addresses review of @dgpv
Addresses review of @kristapsk
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.
Watchonly wallets use pubkeys instead of privkeys, but in a bit of
hack the functions previously called "_get_priv_from_path" would
actually return public keys for watchonly wallets. This could have
pretty terrible consequences one day, so functions like that have
been renamed to use the word "key" instead, which could be either
private or public.
Fidelity bond wallets are intended to be used when at a later date
using fidelity bonds to greatly increase joinmarket's resistance to
sybil attacks. This commit adds support for timelocked addresses.
It allows users to optionally create wallet which support such
addresses. The synchronization code is modified to also scan for
timelocked addresses. The keypairs of the timelocked addresses go in
the newly created 2nd address type, where before the zeroth index were
receive addresses and first index was change.
The locktime dates are fixed at the first of each month for the next 30
years. This means users dont need to remember any dates, and so just
their seed phrase and wallet type will still be enough to recover all
funds. Each keypair used for timelocking requires an additional 360
addresses to be scanned for, which isn't a problem for Bitcoin Core.
Fidelity bonds are only stored in the zeroth mixdepth, as they are not
used in repeated coinjoins so theres no point having them in multiple
mixdepths.
Timelocked addresses don't use the get_new_script() family of functions
because they all assume that one index equals one address, and that
therefore it's possible to ask for a "next" address. For timelocked
addresses knowing the index is not enough to know the address, the
timestamp must be known too. Also once one address made of
(index, timestamp) is used you mustn't use that index and pubkey again,
even though all the other timelocks for that index/pubkey are unused.
This is for privacy reasons, as its equivalent to address reuse.
Previously an example of a BIP32 path
would be: m/wallet-type'/mixdepth'/internal/index
The 'internal' name referred to internal and external addresses (also
called change and receive). The renaming to 'address_type' is in
preparation to add more branches for timelocked addresses and burner
outputs.
The variable formally known as 'internal' is now no longer a boolean
but always an integer. This almost-always seemlessly fits because the
values False and Ture correspond to 0 and 1. The function
_get_internal_type therefore has no purpose anymore. Delete it.
Those names as confusing. They could imply that the function obtains
a path or address given a script. To help the code be more
self-documenting I add the verb from.
The above commit introduces auto freezing for utxos below
a threshold, but erroneously auto freezes new utxos in
almost all cases because transactions are processed
multiple times (add_utxos handles this in the wallet).
The problem here is solved as with the same issue of
duplication in the logging of new transactions; we keep
track of new txids that arrive in the wallet and make
sure not to process the same txid twice.
Additionally, the setting of WalletService.used_addresses
is fixed. Test for this function is also fixed.
Closes#274. Utxos are disabled if they are sent
to a reused address, and are below a threshold
set by the value `max_sats_freeze_reuse` in the
`POLICY` section of the config file. If the value
is -1, such utxos are always frozen irrespective of
the value.
Users are prompted with a warning level logging message
on CLI and a popup on Joinmarket-Qt. Such disabled utxos
can of course be re-enabled by the existing methods.
Also adds test case for address reuse freezing function.
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.
Fixes#469. Prior to this commit, using the now default
version of sync (earlier was called "fast sync"), imports
of addresses beyond those already used in the application
was not occurring, resulting in addresses displayed for
deposit that had not been imported as watch-only in Bitcoin
Core. This meant that a user may deposit but not see the
balance in Joinmarket.
This fix ensures that every address displayed (via any
interface) will always have been pre-imported).
Account movement transactions are deprecated in Core but they still
sometimes appear in old wallets. Such transactions create an entry
in the listtransactions result which doesnt have a "txid" key.
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.
Adam Gibson
Kristaps Kaupe
jules23
chris-belcher
James Hilliard