isDataOwner is used when deciding how many peer nodes should receive
a BroadcastMessage. If the BroadcastMessage originated
on the local node it is sent to ALL peer nodes with a small delay.
If the node is only relaying the message (it originated on a different
node) it is sent to MAX(peers.size(), 7) peers with a delay that is
twice as long.
All the information needed to determine whether or not the
BroadcastMessage originated on the local node is available at the final
broadcast site and there is no reason to have callers pass it in.
In the event that the sender address is not known during broadcast (which
is only a remote possibility due to how early the local node address
is set during startup) we can default to relay mode.
This first patch just removes the deep parameters. The next will remove
everything else. There is one real change in LiteNodeNetworkService.java
where it was using the local node when it should have been using the
peer node. This was updated to the correct behavior.
Now that more callers have moved internal, the public facing API
can be cleaner and more simple. This should lead to a more maintainable
API and less sharp edges with future development work.
Now that the only user is internal, the API can be made private and the
tests can be removed. This involved adding a few test cases to
processGetDataResponse to ensure the invalid hash size condition was
still covered.
The only two users of this constructor are the fromProto path which
now creates an empty Capabilities object similar to GetDataResponse.
The other internal usage of Capabilities.app which is initialized to empty.
The only two users of this constructor are the fromProto path which
already creates an empty Capabilities object if one is not provided and
the internal usage of Capabilities.app which is initialized to empty.
Remove the @Nullable so future readers aren't confused.
Checking for null creates hard-to-read code and it is simpler to just
create an empty set if we receive a pre-v0.6 GetDataResponse protobuf
message that does not have the field set.
Write a few integration test that exercises the exercise interesting
synchronization states including the lost remove bug. This fails
with the proper validation, but will pass at the end of the new feature
development.
Previously, multiple handlers needed to signal off one global variable.
Now, that this check is inside the singleton P2PDataStorage, make it
non-static and private.
Now that we want to unit test the GetData path which has different
behavior w.r.t. broadcasts, the tests need a way to verify that
state was updated, but not broadcast during an add.
This patch changes all verification function to take each state update
explicitly so the tests can do the proper verification.
Introduce a generic function that can be used to filter
Map<ByteArray, PersistableNetworkPayload> or
Map<ByteArray, ProtectedStorageEntry>.
Used to deduplicate the GetData code paths and ensure the logic is the
same between the two payload types.
Move the capability check inside the stream operation. This should
improve performance slightly, but more importantly it makes the
two filter functions almost identical so they can be combined.
The appendOnlyDataStoreService and map already have unique keys that
are based on the hash of the payload. This would catch instances
where:
PersistableNetworkPayload
- None: The key is based on ByteArray(payload.getHash()) which is the
same as this check.
ProtectedStorageEntry
- Cases where multiple PSEs contain payloads that have equivalent
hashCode(), but different data.toProtoMessage().toByteArray().
I don't think it is a good idea to keep 2 "unique" methods on
payloads. This is likely left over from a time when
Payload hashCode() needed to be different than the hash of
the payload.
Remove the dependence on the connection object by having the handler
pass in the peer's capabilities. This now allows unit testing of
buildGetDataResponse without any connection dependencies.
Move the logging that utilizes connection information into the request
handler. Now, buildGetDataResponse just returns whether or not the list
is truncated which will make it easier to test.
Changed the log to reference getDataResponse instead of getData. Now
that we might truncate the response, it ins't true that this is exactly
what the peer asked.
These are identical test cases to the requestHandler tests, but with much
fewer dependencies. The requestHandler tests will eventually be deleted,
but they are going to remain throughout development as an extra safety
net.
As part of changing the GetData path, we want to move all creation
and processing of GetData messages inside P2PDataStorage. This will allow
easier unit testing of the behavior as well as cleaner code in the
request handlers that can just focus on nonces, connections, etc.
We had a small memory leak in the code base. Namely, there have been some
threadpools in use but not shutdown when they have been no longer needed.
Result was that the threads and the parent threads have been kept alive
which lead to hundreds of stale threads over the course of several days.
Problem: previously, in order to completely shut down a running
localnet, users had to attach to their 'localnet' screen and kill (^C)
each process, then quit and kill the entire screen session.
Solution: this change introduces an 'undeploy' target that automates
sending the ^C to each screen window followed by sending screen's 'kill'
command to any remaining windows, thus killing the entire 'localnet'
screen session.
The result is that users may now run the following two commands in
succession any number of times to bring their localnet up and down (to
'deploy' and 'undeploy' their localnet).
# bring up localnet
$ make deploy
# use localnet to test, develop, etc...
# bring down localnet
$ make undeploy
* Add zh-hant translations (Chinese Traditional)
* Add Chinese Traditional file to update_translations.sh script
ACKs for top commit:
@ripcurlx:
ACK 4dab4fc25e
* Fix language tags so script and regional variants work correctly
* Adjust update_translations.sh language tags for script/regional variants
ACKs for top commit:
@ripcurlx:
ACK 089232716d
In commit 5fb4b21 ("Refine deploy target..."), the 'build' target was
made normal, i.e. non-phony, but on further review it does in fact make
sense to declare 'build' phony, such that it is run no matter the status
of the root-level 'build' directory, but for different reasons.
Previously, we had been considering the presence of 'build' directory as
a reasonable proxy for determining whether the `./gradlew build` had
been run. If the directory was present, we considered the 'build' target
up-to-date. If not, then we would re-run `./gradlew build`. This is all
sensible enough, except for the fact that the root-level 'build'
directory has almost nothing to do with the actual output of `./gradlew
build`. Gradle does output 'build' directories, but in the respective
subdirectory for each module of the project. After `./gradlew build` has
been run, we would see a 'desktop/build' directory, a 'seednode/build'
directory and so forth. It just so happens that a root-level 'build'
directory was getting created at all due to idiosyncracies of a
particular Kotlin plugin.
This commit updates the makefile to better respect this reality by:
- preserving the 'build' target but marking it once again as PHONY
- introducing new 'seednode/build' and 'desktop/build' targets that
trigger './gradlew :seednode:build` and ./gradlew :desktop:build`
commands respectively.
- making 'build' depend on these two new targets
In light of this realization of flawed thinking about the root-level
build dir, this change also restores `make clean` to calling `./gradlew
clean` instead of `rm -rf build`.
