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

refactor(transaction): Implement canonical ordering in TransactionPlan #3467

Merged
merged 24 commits into from
May 6, 2024

Conversation

TalDerei
Copy link
Collaborator

@TalDerei TalDerei commented Dec 4, 2023

References #3435 and #3379

@cronokirby
Copy link
Contributor

We should also have tie breaking based on the effect hash, rather than just using whatever order between actions that happens to exist (or perhaps something more intricate). Because of this, it might be a good idea to instead shift the approach of this PR to adding an Eq and Ord instance to Action based on this philosophy.

@hdevalence
Copy link
Member

Let's pause work on this until having a better spec of what should be done, it's relatively less important than other changes we should be focusing on right now (getting new builder code integrated into web extension, making breaking changes before next testnet release).

@TalDerei
Copy link
Collaborator Author

TalDerei commented Dec 4, 2023

This PR is blocked and will remain as a draft until after the next release.

@conorsch
Copy link
Contributor

conorsch commented Dec 7, 2023

We are not planning to merge this for Testnet 64, so we'll return to it next week.

@TalDerei
Copy link
Collaborator Author

TalDerei commented Mar 3, 2024

Is this something we want done for the next testnet release? otherwise it can remain in the backlog

@hdevalence
Copy link
Member

Let's keep it in the backlog for now

@TalDerei TalDerei added the C-design Category: work on the design of Penumbra label Mar 22, 2024
TalDerei and others added 2 commits March 22, 2024 13:19
Signed-off-by: Tal Derei <70081547+TalDerei@users.noreply.github.com>
TalDerei and others added 2 commits May 5, 2024 14:06
Signed-off-by: Tal Derei <70081547+TalDerei@users.noreply.github.com>
@TalDerei TalDerei marked this pull request as ready for review May 5, 2024 21:36
@TalDerei TalDerei requested a review from hdevalence May 5, 2024 21:36
@TalDerei TalDerei added the A-client Area: Design and implementation for client functionality label May 5, 2024
@aubrika aubrika added this to the Sprint 6 milestone May 6, 2024
Copy link
Member

@hdevalence hdevalence left a comment

Choose a reason for hiding this comment

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

LGTM!

@hdevalence hdevalence merged commit f0e7696 into main May 6, 2024
13 checks passed
@hdevalence hdevalence deleted the transaction-ordering branch May 6, 2024 21:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-client Area: Design and implementation for client functionality C-design Category: work on the design of Penumbra
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants