Skip to content
This repository has been archived by the owner on Aug 23, 2020. It is now read-only.

Change bundle validity rules for enabling more robust node architecture #1746

Closed
luca-moser opened this issue Feb 6, 2020 · 4 comments · Fixed by #1786
Closed

Change bundle validity rules for enabling more robust node architecture #1746

luca-moser opened this issue Feb 6, 2020 · 4 comments · Fixed by #1786
Assignees
Milestone

Comments

@luca-moser
Copy link
Member

Description

(I've been instructed to write this issue (and not an RFC))

Enforce that bundles:

  • must only approve tail transactions
  • all transactions within the bundle approve via their branch the trunk transaction of the head transaction.

Motivation

About rule 1

The ontology concept describes an abstraction which allows a node software to create different views from the origin Tangle, in order to enable graphs or data structures to simplify processing transactions, bundles or other data in a meaningful way.

For example, consider a bundle tangle which itself is represented by a graph made up of Bundle objects holding for example fields like this:

  • tail hash
  • diffs on addresses (meaning the address with the corresponding mutation caused by this bundle)
  • refs (a set of hashes to other approved Bundles)
  • ...

Out of such objects, a graph can be built which is basically an abstraction providing immediate information which can be used for tip-selection or other functionalities. In such "bundle tangle", an invalid bundle for example wouldn't even occur, making tip-selection easier (and faster) as the graph represents a validated graph of bundles (meaning the graph only grows when a new valid bundle becomes solid). One can take this ontology concept even further and for example construct a walker tangle out of the bundle tangle, where the graph represents only the subset of vertices which are relevant for tip-selection (for example by removing anything below max depth).

In order to make such bundle tangle ontology work however, it is required that bundles only approve other tail transactions, as otherwise it is not possible to construct such graph: if a bundle could approve a non tail transaction, it wouldn't be clear what the actual tail of that bundle is (remember, one can reattach a tail directly onto the origin bundle infinite times).

About rule 2

Enforcing that a bundle can max approve two other transactions enables the node software to work with less logic or even removes potential vulnerabilities, as it eases the complexity of allowing a lot of cones to be approved by a single atomic transfer. Our client libraries already build bundles in such fashion per default, so it would make sense to let the node software also be only agnostic to such bundles.

Requirements

In order to enforce this rules, the bundle validation logic has to be adjusted to deem bundles invalid where:

  • the branch transaction hash of the non head transactions within a bundle, is not the same as the trunk transaction hash of the head transaction.
  • the trunk and branch transactions approved by the bundles are non tails.

Note that the second validation logic enforces, that a bundle can only be validated when it becomes solid (which shouldn't really change anything as a non-solid bundle in itself is also useless).

Open Questions (optional)

Rule 2 doesn't directly affect the ontology concept, however, it allows it to keep the same "a vertex approves two other vertices", as a Bundle object in such bundle tangle would only need to keep max two other references.

Am I planning to do it myself with a PR?

No.

@kwek20 kwek20 changed the title Change bundle validity rules for enabling robuster node architecture Change bundle validity rules for enabling more robust node architecture Feb 7, 2020
@GalRogozinski GalRogozinski added the L-Groom This issue needs to be groomed label Mar 2, 2020
@GalRogozinski
Copy link
Contributor

We think to answer the open question that rule 2 is a good idea :-)

@GalRogozinski
Copy link
Contributor

Additional requirement:
Write the milestone index in this issue that was first issued with this change on the compass code

@GalRogozinski GalRogozinski removed the L-Groom This issue needs to be groomed label Mar 3, 2020
@GalRogozinski GalRogozinski added this to the Doula milestone Mar 3, 2020
@acha-bill acha-bill self-assigned this Mar 10, 2020
@GalRogozinski
Copy link
Contributor

Bill's PR is good

assigning this to self because I will be making a few changes and releasing this

@GalRogozinski GalRogozinski self-assigned this Mar 16, 2020
@GalRogozinski GalRogozinski modified the milestones: Doula, Corona Mar 16, 2020
@acha-bill
Copy link
Contributor

Did manual testing for rule 1 and 2 by creating funny bundles to test different edge cases.

rule 1

trunk = tail, branch = tail: valid
branch = trunk = non tail: invalid
trunk = non tail, branch = non tail: invalid
trunk = tail, branch = non tail: invalid bundle

For rule 2, created a bundle with 3 txs where txs[1].branch != txs[2].trunk: invalid
Also manually tested that local snapshots still work fine.

I also added unit tests for these.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants