See #1435.
Prior to this commit, if connections were immediately closed,
no backoff or jitter were applied to delay the next connection attempt,
resulting in too many connection attempts to directory nodes that the
client could not reach, and spam in the logs.
After this commit, we enforce that we always shut down the ClientService
on any disconnection trigger or connection failure, and only attempt to
reconnect with a manual backoff delay (4 seconds increasing by 50% for
20 attempts, plus a few seconds jitter), for directory nodes, while for
non-directory nodes we always give up on any failure. Max delay is a bit
above 3 hours.
Prior to this commit, if a message (like a disconnection control
message), was to be sent to live peers, regarding another peer that did
not have a nick field set, a crash would occur in directory nodes
sending this message, because the function
OnionPeer.get_nick_peerlocation_ser() assumed the existence of the nick
field, and raised the OnionPeerError exception if it did not exist.
After this commit, the raising of this Exception is caught when calling
OnionPeer.get_nick_peerlocation_ser, so that a peer in this state is
just not added to the list of peers to be sent to live peers. This
condition is caused specifically by the fact that said peer has not
completed a handshake (which must always pass the value of the nick),
and so the other live peers will not yet be aware of it.
Prior to this commit, we timed out the attempts to connect to individual
directory nodes after 10s, which proves to be too optimistic for
connections to onions, as failures are often seen. After this commit, we
allow 60s (note that this time includes handshake completion).
Fixes#1306.
Prior to this commit, it was possible (unlikely) for a peer to exist in
the active_directories nick in OnionMessageChannel, but to have False
entries (i.e. disconnected) for every directory, meaning that the random
choice from those directories raised an Exception which was not caught,
causing a crash.
This is now fixed by checking whether the list of directories to be
chosen from randomly, is empty, and if so raising the correct Exception
type, namely OnionDirectoryPeerNotFound.
Fixes#1259.
This is done to support envisaged experimentation with significantly
larger counterparty counts than the current usually 10-12 maximum. IRC
notionally supports very large messages because we split messages into
chunks, but realistically, throttling usually causes
timeouts/interruptions if we try to use extremely large amounts of text.
In Joinmarket's communication protocol, the only time we need very large
messages is specifically to allow transfer of fully signed transactions
that need to be pushed to the bitcoin network. (See JMPushTx). So this
bump to the maximum allowable line length is specifically to support
that message, in case people want to try coinjoins with 20-40
participants. The comment indicates an extremely rough guesstimate of
maximum supported counterparty count based on this limit.
Note that users can always just broadcast the final tx themselves, of
course.
Prior to this commit, passive observation of orderbook updates by bots
such as ob-watcher was not able to keep track of nicks/bots that had
disconnected and were no longer available, because non-directory peers
do not get the network level disconnection event.
After this commit, the directory nodes forward a variant of the
"peerlist" control message, that was previously used only to send
connection information from one non-directory peer to another (so that
they could establish a new p2p connection if desired). Now a new version
of that message adds an extra "D" field to indicate that this message is
being used to inform that the relevant peer has disconnected from this
directory node.
Bubbling up the on_nick_leave event: it is already the case that the
final on_nick_leave callback is only triggered by the disappearance of a
nick from all available message channels (so that as long as a nick is
perceived as being in at least one available message channel, its
current offers are still maintained in the local database); this logic
is now repeated one layer down, because the disconnection of a nick from
one directory node may not mean it is shut down, so we only pass the
on_nick_leave callback from the OnionMessageChannel object up to the
MessageChannelCollection callback when the active_directories dict
indicates that this particular nick is no longer available on any of our
configured directory nodes.
Original testing setup of onion message channels just sent messages on
the first directory node peer in our list. This fixes that TODO.
After this commit, we keep a map of which directory nodes we have seen a
given Joinmarket nick on, and we keep track of whether those directory
nodes are available (connected) or not. Then when we want to privmsg
that nick, if we do not currently have a direct connection, we choose
randomly from the set of directory nodes on which that nick has been
seen, and which are currently live/connected.
Prior to this commit, a taker bot that received the orderbook on the
onion message channel would attempt to connect to every maker bot that
had sent an offer, which is not just inefficient but appears to trigger
rate limiting in Tor itself, throwing errors in the SOCKS proxy, at
numbers of around 80 makers, in testing (but could easily happen at
lower levels).
After this commit, we only opportunistically attempt to connect to
makers (using the same OnionPeer.try_to_connect method as before) when
we have already decided to send them a message (e.g. chosen to fill
their offer). This is done by calling try_to_connect only in
OnionMessageChannel._privmsg, and only to the right kind of peer.
The downside of this is that we will send more messages via directory
(at least one additional, for each maker), but clearly the alternative
is not viable.
As part of this update, we ensure that connection attempts are not
accidentally duplicated (see OnionPeer.connecting).
Prior to this commit, running ob-watcher with
an onion-type message channel configured would result
in the bot attempting to connect to all the makers,
which is bad. This happened because the internal logic
of the onion message channel is that receivers of privmsgs
will be sent peer info from the directory, so that they can
immediately respond p2p if they succeed in outward connecting.
But for bots who do not intend to engage in a coinjoin interactive
protocol, like ob-watchers, this is absolutely not the desired outcome.
After this commit, a bot can specify mode "PASSIVE" in the call to
get_mchannels(), which results in the OnionMessageChannel object only
creating non-directory remote peer objects of type OnionPeerPassive,
instead of OnionPeer, which means they never try to connect to those
remote peers.
Also, exports JMMakerClientProtocol for custom directory node scripts
(stored in the custom-scripts repo).
Modify default config with 2 signet and mainnet directory nodes to
start.
Handles unreachable directory nodes with a human readable error and
adjusts connection timeouts to be realistic.
Changes wording in Qt notifications from "IRC" to message channel.
Updates docs, new directory node information.
In the previous commit, all peers served an onion.
After this commit, taker client instances will automatically
send a config var to the jmdaemon backend that instructs
the OnionMessageChannel instance to not start an onion service,
and the handshake messages sent by these peers replace the
onion location with a placeholder string NOT-SERVING-ONION.
Directories and maker peers will not therefore to connect outbound
to them, but privmsging still happens p2p with connections from
takers to makers after the directory has communicated their
reachable .onion addresses.
This change reduces the configuration requirement for takers and
is better for their privacy and security (without sacrificing
the gain we get from having p2p connections).
The above comments re: takers also apply to ob-watcher bots.
This commit also fixes a large number of minor bugs and errors in
documentation, as well as many Python cleanups after review from
@PulpCattel. A few concrete items are:
It fixes the ob-watcher functionality to work with the new subclass
of MessageChannel (OnionMessageChannel).
It corrects the on_nick_leave trigger to make dynamic nick switching
between MessageChannels (as implemented in MessageChannelCollection)
work correctly.
It corrects the order of events in the add_peer workflow to ensure that
a handshake can always be sent so that the activation of the connection
always works.
It sets a default messaging config with onion, 2 active IRC servers and
one inactive IRC server. The onion config has 2 signet directory nodes,
so this will change to mainnet after the PR is merged to master.
Joinmarket bots run their own onion services allowing inbound connections.
Both takers and makers connect to other makers at the mentioned
onion services, over Tor.
Directory nodes run persistent onion services allowing peers to
find other (maker) peers to connect to, and also forwarding
messages where necessary.
This is implemented as an alternative to IRC, i.e. a new
implementation of the abstract class MessageChannel, in onionmc.py.
Note that using both this *and* IRC servers is supported; Joinmarket
supports multiple, redundant different communication methods,
simultaneously.
Messaging is done with a derived class of twisted's LineReceiver,
and there is an additional layer of syntax, similar to but not the
same as the IRC syntax for ensuring that messages are passed with
the same J5.. nick as is used on IRC. This allows us to keep the
message signing logic the same as before. As well as Joinmarket line
messages, we use additional control messages to communicate peer lists,
and to manage connections.
Peers which send messages not conforming to the syntax are dropped.
See https://github.com/JoinMarket-Org/JoinMarket-Docs/pull/12 for
documentation of the syntax.
Connections to directory nodes are robust as for IRC servers, in
that we use a ReconnectingClientFactory to keep trying to re-establish
broken connections with exponential backoff. Connections to maker
peers do not require this feature, as they will often disconnect
in normal operation.
Multiple directory nodes can and should be configured by bots.
Prior to this commit, the file commitmentlist, which stores
commitments (podle) for makers to let them blacklist/prevent
reuse was stored in the local directory for the script, which
allowed remote running of jmdaemon but was very unhelpful for
any situation where multiple bots are running at once, e.g.
in testing or using multiple wallets against the same codebase.
This could result in incorrect rejection of coinjoins.
After this commit, by default, the commitmentlist file is stored
in datadir/cmtdata/commitmentlist, so it will be local to any
custom data directory as would be the case for running multiple
wallets on the same machine. A user can set the POLICY variable
commitment_list_location to "." to revert to the previous behaviour.
Previously, the default config file generated by JM causes an error due
to not having `socks5_host` and `socks5_port`, which are commented out
since `socks5` is disabled by default. This commit makes those fields
optional when `socks5` is not set to `true` in the config. It also
makes `socks5` default to `false` if it is not specified in the config
for a given IRC server.
Fixes#1069.
Before this commit, a non-hex encoded pubkey sent
as part of the privmsg signature raised an Exception,
this commit fixes this, checking the encoding of both
the pubkey and signature fields before sending them
from daemon to client.
Prior to this commit, it was possible for a modified
taker client to get a maker to participate in a join
tx with a size less than their minsize, which could
burn up to an amount of the txfee contribution to
miner fees.
After this commit, the maker stops the coinjoin
coordination when the too-small coinjoin size is seen,
as intended.
Additionally, a check is now in place in the function
`Maker.verify_unsigned_transaction` that the earned
amount is positive, which otherwise could raise an
Exception (but should now be impossible in any case).
Also, changed the default txfee_contibution to zero
as it is of no value to have a nonzero amount.
Also return added for unrecognized orderid, for clearer
error messages.
Fixes#1027. Previous to this commit, the onion
service used by the payjoin receiver was automatically
served on localhost (at an arbitrary port), after this
commit these values can be specified in the PAYJOIN
section of the config.
1. Moves the JMWalletDaemon service class into
the jmclient package (see the wallet_rpc.py module).
2. Adds dependencies "klein" and "autobahn" to the
jmclient package, as well as "pyjwt".
3. Adds another module websocketserver.py, using
autobahn, to allow the JMWalletDaemon service to
serve subscriptions over a websocket, for e.g.
transaction notifications.
4. Adds tests both for the websocket connection
and for the JSON-RPC HTTP connection.
JmwalletdWebSocketServerFactory.sendTxNotification
sends the json-ified transaction details using
jmbitcoin.human_readable_transaction (as is currently
used in our CLI), along with the txid.
Also adds a coinjoin state update event sent via
the websocket (switch from taker/maker/none).
Require authentication to connect to websocket.
5. Add OpenApi definition of API in yaml;
also auto-create human-readable API docs in markdown.
6. Add fidelity bond function to API
7. Add config read/write route to API
8. Remove snicker rpc calls temporarily
9. Updates to docoinjoin: corrects taker_finished
for this custom case, does not shut down at end.
10. Address detailed review comments of @PulpCattel.
Uses Klein to provide HTTP server support.
Adds cookie based auth to requests (made JWT token
based in later commits).
Basic routes are: /unlock, /lock, /display,
/create of wallet.
Encapsulates WalletDaemon as a Service
Add snicker receiver service start, stop
Adds yg/maker function as stoppable service.
Adds a JMShutdown command to
the AMP protocol, allowing a clean shutdown
of a long running bot (e.g. maker) by shutting
down its message channel connections, without
shutting down the entire process.
Adds payment(direct send) request, first draft
Fixes#899.
Before this commit, the dust threshold used to filter
orders in `jmdaemon.OrderbookWatch` was different than
that used in `jmclient`. This is corrected by setting
the threshold in client config and passing this as a
parameter in the `JMInit` command (since the daemon
does not know the client config).
If a yield generator is run with a fidelity bond wallet then the
most-valuable bond will be found and announced.
The announcement includes a proof of a UTXO and its locktime. Also a
proof that the maker's IRC nickname controls the UTXO.
There is also an intermediate signature called the certificate
signature which can later be used when holding fidelity bond UTXOs in
cold storage without the protocol needing to change. Right now this
feature is unused so certificates are generated dynamically on each
request. The certificates have an expiry block height, which is defined
as the number of 2016-block retargeting periods since the genesis
block, so to check if the expiry was passed the taker will check
`current_block_height > cert_expiry*2016`.
Parse incoming and announce outgoing fidelity bond messages
Fidelity bond proof messages will be checked and added to the internal
database just like offers. Such messages are not announced in public
but only directly to takers who ask for them, this is because the
signature proofs must commmit to the maker's and taker's IRC nicknames
in order to avoid replay attacks.
The module socks.py in the jmdaemon package
is no longer used as the SOCKS proxy functionality
is provided by the txtorcon package. This commit
removes it.
This completes the task of enabling
network isolation by running the receiver
side using a hidden service in the daemon,
and communicating over AMP, as is already
the case for the sender.
Updates test_payjoin for daemon receiver.
Qt BIP78 receiver update for daemon.
This PR creates a client-daemon protocol for
the BIP78 sender, using the base protocol
`HTTPPassThrough` which provides tor and non-tor
agents with POST and GET request functionality.
As for Joinmarket coinjoins, the use of an in-process
daemon is the default option, but it can be isolated
by changing the `[DAEMON]` section of the config.
The receiver side of BIP78 will be addressed in a
future PR.
This commit implements a command line script and a GUI
dialog to receive a payment using the BIP78 protocol,
by setting up an ephemeral hidden service.
It also deprecates the pre-existing inter-Joinmarket
protocol for payjoin payments, since we now have
both sending and receiving support for BIP78. Thus,
much code in Maker, Taker and client-daemon protocol
is removed, as is some documentation in docs/PAYJOIN.md.
Also the script `sendpayment.py` is altered to support
only the BIP78 variant.
The test in jmclient/test/test_payjoin now implements
BIP78 over a TCP connection, while the custom tests in
test/payjoinserver.py can support hidden service based
tests, but the latter is not included in the test suite
and may not always work (it is only for manual
investigations).
The following features of BIP78 are supported:
minfeerate
additionalfeeoutputindex - but *only* for single
change output transactions
maxadditionalfeecontribution
The receiver does not have nor request payment
output substitution.
Utxo selection is no longer sophisticated, instead
we only choose a single utxo to keep the size
increase of the transaction minimal. Thus UIH is
not addressed at the moment.
Errors returned are in line with BIP78.
Sequence numbers are checked by receiver, and
kept identical if uniform, otherwise respected.
Receiver uses transaction monitor to shut down
when the payment is seen.
The workflow is almost entirely implemented in
jmclient/payjoin.py and the command line script
is in scripts/receive-payjoin.py. The setup, including
configuration changes for Tor, are documented in
docs/PAYJOIN.md, including a user guide video linked.
Prior to this commit, users setting the POLICY config
option `tx_broadcast` to anything other than `self` would
cause a crash after the merge of #536 due to a bin/hex
conversion failure (before this merge, the tx would simply
fail to broadcast).
This commit adds a `JMTXBroadcast` AMP command so that makers
can send arbitrary transactions from daemon to client, for
broadcast via the blockchain interface. This allows the
existing code in `taker.push()` to function correctly, after
fixing the bin/hex conversion bug. Hence users can now select
`random-peer` or `not-self` and the transaction will be
broadcast as expected according to the comments, and the
WalletService will react to the broadcast just as it does
currently for self-broadcast.
Note that this change will be ineffective if the counterparties
do not support it; the transaction will simply remain un-broadcast.
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
Prior to this commit, the callback method in the
DaemonServerProtocol, on_commitment_seen, was using
a reference to jm_single (to access the config var
accept_commitment_broadcasts) which was not valid
as jm_single() is part of jmclient and is not accessible
to jmdaemon. This error was being swallowed by a finally:
block in the message channel method check_for_commitments,
resulting in public broadcast hp2 messages being ignored.
This commit removes the reference to accept_commitment_broadcasts,
thus resulting in all publically broadcast hp2 messages being
stored in the commitmentlist.
Kristaps Kaupe
Adam Gibson
bisqubutor
Adam Gibson
Daniel McNally
undeath
chris-belcher
Jules Comte