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.

Sign up for GitHub

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

Jump to bottom

fix!: don't include duplicate unique ID in same block template #3713

Merged
merged 5 commits into from
Jan 21, 2022
Merged

fix!: don't include duplicate unique ID in same block template #3713

merged 5 commits into from
Jan 21, 2022

Conversation

sdbondi
Copy link
Member

@sdbondi sdbondi commented Jan 18, 2022 •
edited
Loading

Description

  • Keeps track of unique IDs in the current unconfirmed transaction set
  • Mempool will not select unique IDs for block templates
  • Add basic mempool unit test
  • Implement consensus encoding for remaining output features fields/structs (consensus breaking)
  • Update cucumber JS encoding and allow unique id to be set for transactions
  • Add cucumber test
  • Adds new unit tests for output features consensus encoding

Motivation and Context

A duplicate unique ID in the same block is invalid and will be rejected by the network, since nothing has changed in the mempool after the rejected block the same transactions could be selected for the next block and so on. For this reason, the base node should not place transactions with duplicate unique IDs in the block template.

How Has This Been Tested?

New unit and cucumber test

@sdbondi sdbondi changed the title fix!: don't include duplicate unique ID in same block fix!: don't include duplicate unique ID in same block template Jan 18, 2022
@sdbondi sdbondi force-pushed the core-mempool-unique-id branch 9 times, most recently from 3d446ab to e4610ae Compare January 19, 2022 05:20
@sdbondi

This comment has been minimized.

Sign in to view
SWvheerden
SWvheerden approved these changes Jan 20, 2022
View reviewed changes
Copy link
Collaborator

@SWvheerden SWvheerden left a comment

Choose a reason for hiding this comment

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

LGTM

@aviator-app aviator-app bot added mq-failed and removed mq-failed labels Jan 21, 2022
@aviator-app aviator-app bot merged commit b501771 into tari-project:development Jan 21, 2022
@sdbondi sdbondi deleted the core-mempool-unique-id branch January 21, 2022 08:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stringhandler stringhandler stringhandler approved these changes

@SWvheerden SWvheerden SWvheerden approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sdbondi @SWvheerden @stringhandler