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

BIP 330: Transaction announcements reconciliation #851

Closed
wants to merge 13 commits into from

Conversation

@naumenkogs
Copy link

naumenkogs commented Oct 9, 2019

This spec is necessary to implement higher-level efficient tx relay protocols (such as Erlay).

Submitted to mailing list in late September:
https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-September/017323.html

naumenkogs and others added 7 commits Sep 25, 2019
bip-reconcil: Switch PD "license" to CC0
Fix mediawiki syntax for italic text
Co-authored-by: Rusty Russel <rusty@rustcorp.com.au>
Update bip-reconcil.mediawiki
bip-reconcil.mediawiki Outdated Show resolved Hide resolved
bip-reconcil.mediawiki Outdated Show resolved Hide resolved
bip-reconcil.mediawiki Outdated Show resolved Hide resolved
@luke-jr luke-jr changed the title BIP for transaction reconciliation BIP 330: Transaction announcements reconciliation Nov 4, 2019
bip-330.mediawiki Outdated Show resolved Hide resolved
naumenkogs added 2 commits Nov 4, 2019
bip-0330.mediawiki Outdated Show resolved Hide resolved
@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Nov 5, 2019

Need to add to README:

|-
| [[bip-0330.mediawiki|330]]
| Peer Services
| Transaction announcements reconciliation
| Gleb Naumenko, Pieter Wuille
| Standard
| Draft
pull bot pushed a commit to adaptive/bips that referenced this pull request Nov 5, 2019
BIP 330: Transaction announcements reconciliation
ret, y = ret ^ bit * y, mul2(y)
return ret
def create_sketch(shortids, capacity):

This comment has been minimized.

Copy link
@ysangkok

ysangkok Nov 5, 2019

Contributor

would benefit from type annotations

==Rationale==
====Why using PinSketch for set reconciliation?====

This comment has been minimized.

Copy link
@ysangkok

ysangkok Nov 5, 2019

Contributor

Wouldn't "Why use" sound better here? I don't think this sentence is like the second example here.

====q-coefficient====
The q value should be stored to make efficient difference estimation. It is shared across peers and changed after every reconciliation.
q-coefficient represents the discrepancy in sets: the closer the sets are, the lower optimal ''q'' is.

This comment has been minimized.

Copy link
@ysangkok

ysangkok Nov 5, 2019

Contributor

why the inconsistent italics for q?

However, using full 256-bit IDs to uniquely identify transactions seems to be an overkill.
128 is the highest power of 2 which provides good enough collision-resistance in an adversarial model, and trivially saves a significant portion of the bandwidth related to these announcements.
====Why using bisection instead of extending the sketch?====

This comment has been minimized.

Copy link
@ysangkok

ysangkok Nov 5, 2019

Contributor

again, I suggest "use" instead of "using"

A reconciliation set is moved to the corresponding set snapshot after the transmission of the initial sketch.
====Reconciliation set snapshot====
After the transmitting of the initial sketch (either sending or receiving of reconcildiff message), every node should store the snapshot of the current reconciliation set, and clear the set.

This comment has been minimized.

Copy link
@ysangkok

ysangkok Nov 5, 2019

Contributor

I suggest inserting a "the" before "message" in "sending or receiving of reconcildiff message". I suggest "After transmitting the initial sketch" instead of "After the transmitting of the initial sketch".

|-
| uint8 || success || Indicates whether sender of the message succeeded at set difference decoding.
|-
| uint32[] || ask_shortids || The short IDs that the sender did not have.

This comment has been minimized.

Copy link
@ysangkok

ysangkok Nov 5, 2019

Contributor

Again, I prefer code formatting for data types.

{|class="wikitable"
! Data type !! Name !! Description
|-
| uint128[] || inv_truncids || The truncated IDs of transactions the sender believes the receiver does not have.

This comment has been minimized.

Copy link
@ysangkok

ysangkok Nov 5, 2019

Contributor

Again, I prefer code formatting for data types.

{|class="wikitable"
! Data type !! Name !! Description
|-
| uint128[] || ask_truncids || The truncated IDs of transactions the sender wants the full transaction data for.

This comment has been minimized.

Copy link
@ysangkok

ysangkok Nov 5, 2019

Contributor

Again, I prefer code formatting for data types.

====Why using 32-bit short transaction IDs?====
To use Minisketch in practice, transaction IDs should be shortened (ideally, not more than 64 bits per element).
Small number of bits per transaction also allows to save extra bandwidth and make operations over sketches faster.

This comment has been minimized.

Copy link
@ysangkok

ysangkok Nov 5, 2019

Contributor

Should be "A small number of bits" instead of "Small number of bits". "Allows to" is clumsy, I prefer "allows saving" instead of "allows to save"

====Why using bisection instead of extending the sketch?====
Unlike extended sketches, bisection does not require operating over sketches of higher order.
This allows to avoid the high computational cost caused by quadratic decoding complexity.

This comment has been minimized.

Copy link
@ysangkok

ysangkok Nov 5, 2019

Contributor

Ditto, "allows to avoid" should be avoided and replaced with "allows avoiding".

@luke-jr

This comment has been minimized.

Copy link
Member

luke-jr commented Nov 5, 2019

This is merged, but GitHub messed up detecting it..

@luke-jr luke-jr closed this Nov 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.