Skip to content

Remove aggregated branches #2070

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

Conversation

jonastheis
Copy link
Contributor

@jonastheis jonastheis commented Feb 22, 2022

Description of change

To simplify the concepts of the BranchDAG, we remove the AggregatedBranches from the code base and just carry BranchIDs in every object (lots of duplicate branches might be stored but due to merge to master it should be limited and go back to master branch quickly). Later on we can introduce a storage optimization aka CompressedBranchIDs to handle deduplication on the storage layer itself. In that way the interface of Message, Transaction and Output does not change regardless of how the branch IDs are actually stored.

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

@jonastheis jonastheis mentioned this pull request Feb 22, 2022
5 tasks
@jonastheis jonastheis marked this pull request as draft February 22, 2022 20:10
@karimodm karimodm changed the base branch from develop to feat/mtm-aftermath February 23, 2022 09:54
@karimodm karimodm force-pushed the feat/remove-aggregated-branches-replace-simple-branches branch from 7a31b14 to cc32020 Compare February 25, 2022 12:40
karimodm and others added 4 commits February 25, 2022 14:16
…' of github.com:iotaledger/goshimmer into feat/remove-aggregated-branches-replace-simple-branches

# Conflicts:
#	packages/tangle/messagefactory_test.go
jonastheis and others added 6 commits February 25, 2022 13:30
…' of github.com:iotaledger/goshimmer into feat/remove-aggregated-branches-replace-simple-branches
…' of github.com:iotaledger/goshimmer into feat/remove-aggregated-branches-replace-simple-branches
@jonastheis jonastheis marked this pull request as ready for review February 28, 2022 11:34
@jonastheis jonastheis requested review from karimodm and hmoog February 28, 2022 11:44
Copy link
Contributor

@karimodm karimodm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work man, thanks!

karimodm and others added 3 commits February 28, 2022 16:29
…move-aggregated-branches-replace-simple-branches

# Conflicts:
#	pkged.go
#	plugins/webapi/tools/message/diagnostic_messages.go
Copy link
Contributor

@hmoog hmoog left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have left some comments

@jonastheis jonastheis merged commit ba3c485 into feat/mtm-aftermath Mar 4, 2022
@jonastheis jonastheis deleted the feat/remove-aggregated-branches-replace-simple-branches branch March 4, 2022 13:25
jonastheis added a commit that referenced this pull request Mar 4, 2022
@jonastheis jonastheis restored the feat/remove-aggregated-branches-replace-simple-branches branch March 4, 2022 16:29
@jonastheis jonastheis deleted the feat/remove-aggregated-branches-replace-simple-branches branch March 4, 2022 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants