Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove address prefix for mailbox messages #4609

Conversation

chimp1984
Copy link
Contributor

@chimp1984 chimp1984 commented Oct 7, 2020

Remove address prefix in case the peer has updated vesion as well.
Processes all mailbox messages in a thread. (1000 msgs takes about 1 sec).

EDIT: Merged master and rebased
Capability handling is still WIP

Add `boolean contains(Capability capability)` method
Remove debug log, remove annotation
Set address prefix to empty bytes in case we know that peer has capability (updated version)
Batch process mailbox messages in a thread.
Refactor handling of mailbox messages
Make capabilities final

If capability changes we would have had duplicate entries
@chimp1984 chimp1984 force-pushed the remove-address-prefix-for-mailbox-messages branch from 63ce5a6 to fc0082a Compare October 7, 2020 18:35
@chimp1984 chimp1984 changed the title Remove address prefix for mailbox messages [WIP] Remove address prefix for mailbox messages Oct 7, 2020
The check for AckMessage is not needed anymore as we remove the interface from AckMessage
Impl. applyCapabilities

Cleanups
Rename getOptionalPersistedPeer to findPersistedPeer
Improve getConnectedReportedPeers method
@chimp1984 chimp1984 changed the title [WIP] Remove address prefix for mailbox messages Remove address prefix for mailbox messages Oct 8, 2020
@chimp1984
Copy link
Contributor Author

Complete with dev testing...

Copy link
Member

@sqrrm sqrrm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should think about Optional<> as a very short stream and use in the same way we use streams. It flows nicely using ifPresent, orElse and flatMap rather than checking isPresent and then treating it as a value.

Fix incorrectly commented out code (was for dev testing commented out)
chimp1984 added a commit to chimp1984/bisq that referenced this pull request Oct 8, 2020
…w. This is not the case without getting PR bisq-network#4609 merges as well.

We only do it for 2 weeks after planned release time as then it can be assumed that enough nodes have updated that the normal publishing will distribute the object sufficiently.
@chimp1984 chimp1984 mentioned this pull request Oct 8, 2020
@chimp1984
Copy link
Contributor Author

Replaced by #4610

@chimp1984 chimp1984 closed this Oct 8, 2020
@chimp1984 chimp1984 deleted the remove-address-prefix-for-mailbox-messages branch October 8, 2020 23:27
chimp1984 added a commit to chimp1984/bisq that referenced this pull request Oct 8, 2020
…w. This is not the case without getting PR bisq-network#4609 merges as well.

We only do it for 2 weeks after planned release time as then it can be assumed that enough nodes have updated that the normal publishing will distribute the object sufficiently.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants