dashpay / dash

Dash - Reinventing Cryptocurrency
https://www.dash.org
MIT License
1.49k stars 1.2k forks source link

feat: broadcast dsq messages using the inventory system #6148

Closed PastaPastaPasta closed 1 month ago

PastaPastaPasta commented 2 months ago

Issue being fixed or feature implemented

DSQ messages are 142 bytes.

Previously, assuming a relatively highly connected masternode hosting 100 connection, each round of coinjoin will result in 14.2KB (100*142) of inbound and outbound traffic each.

What was done?

Now, using the inventory system, a message will first use 36 bytes per peer (sending and receiving), plus the size of a getdata message and the actual message itself. As a result, bandwidth usage for 1 round of mixing would be closer to 36 * 100 + 142 (dsq) + 36 (getdata) = ~3.8KB, a reduction of around ~73%

How Has This Been Tested?

Has not been; @UdjinM6 especially please review well :)

Breaking Changes

Does introduce a new protocol version, but in a backwards compatible way. I don't think this would need to be delayed to v22 for any reason.

Checklist:

Go over all the following points, and put an x in all the boxes that apply.

PastaPastaPasta commented 1 month ago

rebased on develop

PastaPastaPasta commented 1 month ago

@UdjinM6 can you re-review based on latest?

UdjinM6 commented 1 month ago

@UdjinM6 can you re-review based on latest?

any thoughts on my previous comments? https://github.com/dashpay/dash/pull/6148#pullrequestreview-2196272307

PastaPastaPasta commented 1 month ago

@UdjinM6 can you re-review based on latest?

any thoughts on my previous comments? #6148 (review)

@UdjinM6 what do you mean? I thought I applied all concepts, didn't fix linter yet. Is that all you mean?

UdjinM6 commented 1 month ago

@UdjinM6 can you re-review based on latest?

any thoughts on my previous comments? #6148 (review)

@UdjinM6 what do you mean? I thought I applied all concepts, didn't fix linter yet. Is that all you mean?

Weird... Had to reload a couple of times to see new changes 🤷‍♂️

PastaPastaPasta commented 1 month ago

@UdjinM6 please re-review

UdjinM6 commented 1 month ago

also, clang-format isn't happy :)