feat(sds): send and receive sync messages #2293
Conversation
adklempner
force-pushed
the
feat/sds-sync
branch
from
efc7597 to
cc4bcaf
Compare
size-limit report 📦
|
| }; | ||
|
|
||
| if (callback) { | ||
| return callback(message); |
There was a problem hiding this comment.
is thins callback intended to be used with async operations?
I see it is sync but we don't have any medium of sending message in a sync way.
I might misunderstand something but I believe it is intended to be used like:
node.sds.sendSyncMessage(async () => {
const resp = await node.lightPush.send();
if (resp.success) return true;
return false;
There was a problem hiding this comment.
good point, also curious
another point: this function isnt really "sending", but "preparing" the message, to be sent with lightpush, right?
perhaps doing:
public createSyncMessage(): Message { ... }
const msg = channel.createSyncMessage();
await node.lightPush.send(msg);or if we want to keep the same API, perhaps:
withSyncMessage(callback)There was a problem hiding this comment.
@weboko good point about async, it should support it. The example you provided is more or less how I intend for the library to be used. Updated in latest commit
@danisharora099 Ideally the library is agnostic towards what the consumer does with the message, and for the case of sync messages it's not as important. But for regular messages I do lean towards the assumption that the callback provided actually sends the message, since it gets added to the outgoing buffer.
It's a good question that deserves more consideration. We can choose to make no assumptions about whether the message was sent, and just get a basic confirmation that the consumer "accepted" it (plus generated and returned a waku message hash for non-sync messages). Then it doesn't matter what kind of retry logic is implemented on the Waku layer (so long as the message hash doesn't change) since the periodic outgoing buffer sweep in SDS will detect that the message was never acknowledged, and then notify the consumer.
Either way, I think it makes sense to stick with the callback pattern, mainly because of the retrieval hint that needs to be returned (see #2299 )
| }; | ||
|
|
||
| if (callback) { | ||
| return callback(message); |
There was a problem hiding this comment.
good point, also curious
another point: this function isnt really "sending", but "preparing" the message, to be sent with lightpush, right?
perhaps doing:
public createSyncMessage(): Message { ... }
const msg = channel.createSyncMessage();
await node.lightPush.send(msg);or if we want to keep the same API, perhaps:
withSyncMessage(callback)|
|
||
| const bloomFilter = getBloomFilter(channelA); | ||
| expect( | ||
| bloomFilter.lookup(MessageChannel.getMessageId(new Uint8Array())) |
There was a problem hiding this comment.
the functionality is good, however, here we check if an empty content message ID exists in the filter - perhaps we can fetch an existing message ID like:
expect(bloomFilter.lookup(someRealMessageId)).to.equal(true/false)
adklempner
force-pushed
the
feat/sds-sync
branch
from
cc4bcaf to
13ae5d4
Compare
Problem / Description
Per the specifications, participants should periodically send sync messages. These are sent with empty content and should not be persisted.
Solution
Adds a public function for sending a sync message in the channel.
Updates other functions to properly handle sync messages (e.g. received messages that are determined to be sync messages are not persisited)
Notes
Checklist