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

RFC: Weighted Uniform Random Tip-Selection #8

Merged
merged 20 commits into from Jun 9, 2020

Conversation

luca-moser
Copy link
Member

@luca-moser luca-moser commented Mar 10, 2020

@thibault-martinez thibault-martinez moved this from Draft to Review in Reviewing process Mar 11, 2020
@luca-moser luca-moser changed the title RFC: (almost) URTS on a subset RFC: Weighted Uniform Tip-Selection Mar 11, 2020
@luca-moser luca-moser changed the title RFC: Weighted Uniform Tip-Selection RFC: Weighted Uniform Random Tip-Selection Mar 11, 2020
@gjeee
Copy link

@gjeee gjeee commented Mar 22, 2020

The concept (to me at least) of "confirmed root transactions" is a bit unclear (yellow boxes in the pic). But what are the green ones then? I would think they are as well.
image

Here again...what are the green ones then?
image

@gjeee
Copy link

@gjeee gjeee commented Mar 22, 2020

Also I think TSI is not exactly defined:

Transaction Snapshot Index (TSI) defines the index of the milestone which confirmed a given transaction.

Shouldn't it be: ......the index of the oldest milestone......?

@luca-moser
Copy link
Member Author

@luca-moser luca-moser commented Mar 23, 2020

@gjeee First thanks for your comments, please use the review feature next time as the way you wrote your comments they don't target specific lines (even though they should/do)

Regarding your first comment:

The concept (to me at least) of "confirmed root transactions" is a bit unclear (yellow boxes in the pic). But what are the green ones then? I would think they are as well.

The text from the RFC says:

Confirmed Root Transactions defines the set of first seen transactions which are confirmed by a previous milestone when we walk the past cone of a given transaction. The walk stops on confirmed transactions.

image

The green transactions are not Confirmed Root Transactions because if you would walk as described in the text from the PoV transaction, you wouldn't reach them as you stop walking as soon as you reach an already confirmed Confirmed Root Transaction. Those "stopping points" are the yellow transactions as described in the RFC.

Regarding your second comment:

Also I think TSI is not exactly defined:

Transaction Snapshot Index (TSI) defines the index of the milestone which confirmed a given transaction.

Shouldn't it be: ......the index of the oldest milestone......?

Only ever one milestone does confirm a given transaction, there's no need to say oldest in this context. Of course newer milestone still reference previous transactions but in this context it is clear that we're talking about the milestone which confirms a transaction.

Copy link
Contributor

@GalRogozinski GalRogozinski left a comment

As per discord discussion the following points should be emphasized clearly:

  1. A tip is eligible for selection only if it is solid
  2. An invalid tip (doesn't pass bundle validation) should be given a score 0. As already defined, this score will propagate upwards

Copy link
Member

@thibault-martinez thibault-martinez left a comment

Tiny nit but the README says the path should be text/0000-my-feature/0000-my-feature.md otherwise everything is on the root.

Wollac
Wollac approved these changes Apr 20, 2020
@muXxer
Copy link

@muXxer muXxer commented May 5, 2020

A tip should stay in the tip pool for some time, even if it got referenced by other transactions.
Otherwise the tangle would become really slim and it would be much harder to sync, because we have to request tx by tx.

Maybe have two criteria at the same time? A minimum time after first reference by another tx or a maximum amount of references. If one of the two criteria is reached, the tip is removed from the pool.

@luca-moser
Copy link
Member Author

@luca-moser luca-moser commented May 5, 2020

I've added a paragraph for those rules, please verify whether they are ok @muXxer.

@luca-moser luca-moser requested a review from hmoog May 8, 2020
@GalRogozinski
Copy link
Contributor

@GalRogozinski GalRogozinski commented Jun 4, 2020

2 points we need to agree on:

  1. Only bundles with valid signatures should be legible for tip selection

currently we think yes

  1. Do we need to add a delay before tip before removing from tip pool?

@luca-moser luca-moser requested review from Wollac and GalRogozinski Jun 4, 2020
Wollac
Wollac approved these changes Jun 6, 2020
hmoog
hmoog approved these changes Jun 9, 2020
Copy link

@hmoog hmoog left a comment

LGTM

@luca-moser luca-moser merged commit f003231 into iotaledger:master Jun 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants