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

Expose relay messages and connection state #54

Merged
merged 1 commit into from
Mar 12, 2024

Conversation

jmateoac
Copy link
Collaborator

@jmateoac jmateoac commented Sep 9, 2023

I was able to hook up Nostrino relays into Plasma with these changes. It's still a WIP, but I wanted to start some conversations around a few of the changes

@@ -1,6 +1,6 @@
[versions]
acinqSecp256 = "0.10.1"
guava = "32.1.1-jre"
guava = "31.1-jre"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

My Android builds were hanging on the newer version of guava. (historically, android and guava don't get along very well)
Any appetite for finding a different solution for the cache?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added #57

private val subscriptions: MutableMap<Subscription, Set<Filter>> = Collections.synchronizedMap(mutableMapOf())

private var connectionState = Disconnected
private var _connectionState = MutableStateFlow(Disconnected)
val connectionState : StateFlow<ConnectionState> get() = _connectionState.asStateFlow() // TODO add tests
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is important so we can show users the current state of the relays

Comment on lines 167 to 169
get() = sent.map { event ->
EventMessage(arbSubscriptionId.next(), event)
}.asFlow()
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there a better way to handle the arb relay messages in this test?

Copy link
Collaborator

Choose a reason for hiding this comment

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

sent.asSequence()
  .zip(arbSubscriptionId.samples())
  .map { (event, id) -> EventMessage(id, event) }
  .asFlow()

@jmateoac jmateoac requested review from Synesso and hugomd September 9, 2023 18:09
@@ -52,18 +54,21 @@ abstract class Relay {
/** Unsubscribe from a subscription */
abstract fun unsubscribe(subscription: Subscription)

/** All events transmitted by this relay for our active subscriptions */
/** All messages transmitted by this relay for our active subscriptions */
abstract val relayMessages : Flow<RelayMessage>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Exposing all messages so we can handle them on the app-side.
It's also important to expose the subscription ID where the message was received so that the correct observer knows the message is for them.

Comment on lines +58 to 65
.filterNot {
it is EventMessage && cache.get(it.event.id)
}
.map {
cache.put(it.id, true)
if(it is EventMessage) {
cache.put(it.event.id, true)
}
it
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll have to find a better way to do this.
If multiple subscriptions share the same message, only one of them will receive it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Raised as #58

@jmateoac jmateoac force-pushed the jmateo/expose_relay_events branch from a09a960 to 1486398 Compare March 12, 2024 01:17
@CLAassistant
Copy link

CLAassistant commented Mar 12, 2024

CLA assistant check
All committers have signed the CLA.

@jmateoac jmateoac changed the title WIP Expose relay messages and connection state Expose relay messages and connection state Mar 12, 2024
@jmateoac jmateoac marked this pull request as ready for review March 12, 2024 01:20
@jmateoac jmateoac requested a review from tyiu as a code owner March 12, 2024 01:20
@jmateoac jmateoac merged commit 57bdde7 into main Mar 12, 2024
3 checks passed
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.

3 participants