The notification queue used for websocket client notifications had a
bug which caused the next queued item in some situations to not be
sent, but instead send a previously sent item. This change fixes this
by setting the 'next' variable to the next item which must be
dequeued, if the queue length is non-zero.
This bug did not always manifest itself as if the receiving goroutine
was ready, a queued item could be sent directly to it rather than
waiting in the queue to be sent at a later time.
This commit modifies the RPC server such that all handlers now receive a
channel which will be notified when a client disconnects. This
notification can then be used to stop long-running operations early when a
client disconnects.
This capability was already present for websocket clients, but this commit
exposes it to standard HTTP clients as well.
Also, since the new websoscket package allows the message type to be set
independently from the type of the variable, remove the casts between
strings and []byte in the websocket read/write paths. This avoids extra
copies thereby reducing the garbage generated.
Closes#134.
This change modifies the params struct to embed a *btcnet.Params,
removing the old parameter fields that are handled by the btcnet
package.
Hardcoded network checks have also been removed in favor of modifying
behavior based on the current active net's parameters.
Not all library packages, notable btcutil and btcchain, have been
updated to use btcnet yet, but with this change, each package can be
updated one at a time since the active net's btcnet.Params are
available at each callsite.
ok @davecgh
This change fixes rescan to include transactions that pay to the
pubkey for a rescanned pubkey hash address. This behavior was lost
when the rescan was optimized for specific types of the
btcutil.Address interface.
ok @davecgh
This commit correctly sets the error in the marhsalled reply if it is
already a *btcjson.Error. Previously it would only set the error if it
was not of that type which led to some RPC results showing no error when
they actually had one.
Copying the RIPEMD160 after SHA256 hash result into a new stack array
to be used as a map lookup key can be quite expensive, and this should
be avoided if possible on intensive tasks such as rescans. This
change takes advantage of the new Hash160 methods of the
AddressPubKeyHash and AddressScriptHash types to use the address's
underlying hash array directly, rather than creating a copy from the
ScriptAddress result.
Unfortunately, for AddressPubKey, ScriptAddress may return either a
byte slice of len 33 or 65 depending on whether the pubkey is
compressed or not, so no such straightforward optimization is
possible.
As a result of this change, I have seen rescans perform roughly 3.5x
faster than before.
The websocket extension command to register for notifications when a new
transaction has been accepted to the memory pool and the resulting
notifications have been renamed. This commit catches up to the change.
This change periodically (about every 10 seconds) notifies the
connected websocket client of the height of the last processed block
as part of the rescan. This enables clients to listen for the
notification and keep track of how much progress a rescan has made
even without any results being found. If the websocket connection is
lost during the rescan, on reconnect, clients may safely start over at
the last notified height.
This change adds select statements to each channel write that may
occur from a non-RPCS goroutine to unblock once the server has begun
shutting down. This prevents issues in clean shutdown where non-RPCS
goroutines may block indefinitely on message sends to the RPC server.
This change adds rescan fast paths for all current implementations of
the btcutil.Address interface. Rather than encoding a payment address
string from a txout script, the minimum number of bytes (which depends
on the address type) are copied into a stack array and used as a
lookup key to a map exclusive for that address type. This performs
better than the previous implementation due to avoiding the base58
encoding and generating less garbage (so each GC cycle completes
faster).
If the address type switch falls into the default (unhandled) case,
the encoded payment address is used as a fallback. This keeps the
intended rescan behavior even when new implementations of
btcutil.Address are added without a rescan fast path.
Benchmarks indicate that when a fast path is followed, for 20 byte
RIPEMD-160 hashes (such as P2PKH and P2SH), skipping the payment
address encoding (and thus bypassing the base58 encoding code) can
result in map lookups up to 60x faster.
This change modifies the RPC server's notifiation manager from a
struct with requests, protected by a mutux, to two goroutines. The
first maintains a queue of all notifications and control requests
(registering/unregistering notifications), while the second reads from
the queue and processes notifications and requests one at a time.
Previously, to prevent slowing down block and mempool processing, each
notification would be handled by spawning a new goroutine. This lead
to cases where notifications would end up being sent to clients in a
different order than they were created. Adding a queue keeps the
order of notifications originating from the same goroutine, while also
not slowing down processing while waiting for notifications to be
processed and sent.
ok @davecgh
This commit changes the websocket client code to use a mutex for
disconnect since it's theoretically possible a non-blocking select on the
quit channel could fall through from two different goroutines thus causing
a second call to close.
ok @jrick.
This change improves the mechanism by which btcd notifies a websocket
client of transaction relating to watched address and unspent outputs
in the following ways:
1. The old processedtx notification has been replaced with the new
recvtx notification. This notification, rather than parsing out
details used by wallet clients, sends the serialized transaction
(as hexadecimal) and any block details (if mined) if any transaction
output sends to one of the websocket client's watched addresses.
2. The old txspent notification has been replaced with the new
redeemingtx notification. This notification, rather than parsing
out details used by wallet clients, sends the serialized transaction
(as hexadecimal) and any block details (if mined) if any transaction
input spends a watched output.
3. When processing notifications for transaction outputs, if any output
spends to a client's watched address, a corresponding spent request
is automatically registered.
4. Transaction notifications originating from mempool now include both
transaction inputs and outputs, rather than only processing
5. When processing notifications for transaction inputs, a client's
output spent request is only removed if the transaction being
processed has also been mined into a block. In combination with the
4th change, this results in two redeemingtx notifications for
transactions which first appear in mempool and are subsequently mined
into a block.
Previously the websocket notifications for addresses were limited to
pay-to-pubkey-hash only. This commit removes that restriction so
all btcutil.Address types are supported. This includes pay-to-pubkey,
pay-to-pubkey-hash, and pay-to-script-hash.
When a spent notification and address notification is removed, the
tracking entry in the client which is used to track what to remove on
shutdown needs to be removed as well.
This commit refactors the entire websocket client code to resolve several
issues with the previous implementation. Note that this commit does not
change the public API for websockets. It only consists of internal
improvements.
The following is the major issues which have been addressed:
- A slow websocket client could impede notifications to all clients
- Long-running operations such as rescans would block all other requests
until it had completed
- The above two points taken together could lead to apparant hangs since
the client doing the rescan would eventually run out of channel buffer
and block the entire group of clients until the rescan completed
- Disconnecting a websocket during certain operations could lead to a hang
- Stopping the rpc server with operations under way could lead to a hang
- There were no limits to the number of websocket clients that could
connect
The following is a summary of the major changes:
- The websocket code has been split into two entities: a
connection/notification manager and a websocket client
- The new connection/notification manager acts as the entry point from
the rest of the subsystems to feed data which potentially needs to
notify clients
- Each websocket client now has its own instance of the new websocket
client type which controls its own lifecycle
- The data flow has been completely redesigned to closely resemble the
peer data flow
- Each websocket now has its own long-lived goroutines for input, output,
and queuing of notifications
- Notifications use the new notification queue goroutine along with
queueing to ensure they dont't block on stalled or slow peers
- There is a new infrastructure for asynchronously executing long-running
commands such as a rescan while still allowing the faster operations to
continue to be serviced by the same client
- Since long-running operations now run asynchronously, they have been
limited to one at a time
- Added a limit of 10 websocket clients. This is hard coded for now, but
will be made configurable in the future
Taken together these changes make the code far easier to reason about and
update as well solve the aforementioned issues.
Further optimizations to improve performance are possible in regards to
the way the connection/notification manager works, however this commit
already contains a ton of changes, so they are being left for another
time.
The wsContext was being locked twice when NewBlockNotifyCheckTxIn is
called. Fixed by changing handlers to assume lock is acquired and
renamed methods to not be exported.
Since the websocket handlers run in their own separate goroutines, it's
possible that they execute after the websocket connection has been
closed and cleaned up. This commit add the necessary checks to ensure
stale data isn't added to notification lists and that requests to closed
connections are ignored.
This closes#92.
Access to connections map and associated notification maps in rpcServer
need to be protected with s.ws.Lock to prevent race with add/remove new
clients.
This closes#88.
Changed mempool.MaybeAcceptTransaction to accept an additional parameter
to differentiate betwee new transactions and those added from
disconnected blocks.
Added new fields to requestContexts to indicate which clients want to
receive all new transaction notifications.
Added NotifyForNewTx to rpcServer to deliver approriate transaction
notification.
