This is done primarily for concision. This change also repackages
bisq.grpc => bisq.proto.grpc in anticipation of repackaging the
definitions in pb.proto from 'protobuf' to 'bisq.proto'. There should
not be any compatibility issues with doing this, but it's out of scope
here. When complete, the relationship between bisq.proto.grpc and
bisq.proto will be more intuitively clear, i.e. that bisq.proto.grpc has
certain dependencies on bisq.proto classes, but not the other way
around.
Protobuf definition files were moved from common and core to a new
protodefinition subproject.
The two main reasons for doing this are to speed up builds by not
having to regenerate common and core protobuf classes
every time a change is made in those subprojects, and to remove
the grpc cli's direct dependency on core, and the transitive dependency
on common.
In order to accomplish this, cli's BisqCliMain was stripped of
its dependencies on common and core. Cli can only get the version
and balance now.
gRPC stub boilerplate was moved from BisqCliMain to a CliCommand
class to avoid some of the bloat that is going to happen as the
read-response loop supports more rpc commands.
Provide UserThreadMappedPersistableList subclass for persistable lists
which need to implement UserThreadMappedPersistableEnvelope, instead of
putting the interface on the base class.
Make the (non-storage) classes MeritList and VoteWithProposalTxIdList
keep the original PersistableList superclass, deriving the remaining
subclasses of PersistableList from the new class instead. In this way,
further persistence-related changes are less likely to inadvertently
alter the behaviour of those two consensus-critical classes.
Removing the superfluous PersistableEnvelope interface from the two
classes (via the base class) will be done in a separate PR.
Currently bisq desktop does not accept IPv6 addresses in the settings for
custom nodes or via the --btcNodes command line option. The separation of
address and port is handled incorrectly in core / BtcNodes::fromFullAddress.
This results in IPv6 addresses being ignored. Where Tor is enabled for
Bitcoin connections, we need to handle the IPv6 address response
from Tor DNS lookup.
Fixes#3990
Testing showed that the new mechanic for checking a local BTC node's
configuration is unstable. This commit reverts to just checking if the
relevant port is open. The recent refactoring and centralization of
logic is still in place.
Testing showed that the new mechanic for checking a local BTC node's
configuration is unstable. This commit reverts to just checking if the
relevant port is open. The recent refactoring and centralization of
logic is still in place.
Make the default toPersistableMessage() method of PersistableEnvelope
simply delegate to Proto.toProtoMessage for speed, so that stores can
explicitly implement (Threaded|UserThreadMapped)PersistableEnvelope if
they actually need concurrency control.
As part of this, make PeerList implement PersistableEnvelope directly
instead of extending PersistableList, as it is non-critical & cloned on
the user thread prior to storage anyway, so doesn't need be thread-safe.
In this way, only PaymentAccountList & small DAO-related stores extend
PersistableList, so they can all be made user-thread-mapped.
After this change, the only concrete store classes not implementing
(Threaded|UserThreadMapped)PersistableEnvelope are:
AccountAgeWitness, BlindVotePayload, ProposalPayload, SignedWitness,
TradeStatistics2, NavigationPath & PeerList
The first five appear to erroneously implement PersistableEnvelope and
can be cleaned up in a separate commit. The last two are non-critical.
(Make NavigationPath.path an immutable list, for slightly better thread
safety anyway - that way it will never be observed half-constructed.)
Add toProtoMessageSynchronized() default method to PersistableEnvelope,
which performs (blocking) protobuf serialisation in the user thread,
regardless of the calling thread. This should prevent data races like
the ConcurrentModificationException observed in #3752, under the
reasonable assumption that shared persistable objects are only mutated
in the user thread.
Also add a ThreadedPersistableEnvelope sub-interface overriding the
default method above, to let objects which are expensive to serialise
(like DaoStateStore) be selectively serialised in the 'save-file-task-X'
thread as before, but directly synchronised with each mutating op. As
most objects are cheap to serialise, this avoids a noticeable perf drop
without having to track down every mutating method for each store.
In all cases but one, classes implementing ThreadedPersistableEnvelope
are stores like TradeStatistic2Store, with a single ConcurrentHashMap
field. These require no further serialisation, since the map entries are
immutable, so the only mutating operations are map.put(..) calls which
are already synchronised with map reads. (Even if map.values().stream()
sees updates @ different keys happen out-of-order, it should be benign.)
The remaining case is DaoStateStore, which is only ever reset or
modified via a single persist(..) call with a cloned DaoState instance
and hash chain from DaoStateSnapshotService, so there is no aliasing
risk from the various DAO state mutations done in DaoStateService and
elsewhere.
This should fix#3752.
Minor change for consistency: narrow the signature of some remaining
such methods, which have return type 'PersistableEnvelope'.
(This excludes some other cases with return type 'NetworkEnvelope'.)
