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

Move ActionList from view crate into transaction crate #4332

Closed
hdevalence opened this issue May 6, 2024 · 6 comments
Closed

Move ActionList from view crate into transaction crate #4332

hdevalence opened this issue May 6, 2024 · 6 comments
Assignees
Labels
C-design Category: work on the design of Penumbra needs-refinement unclear, incomplete, or stub issue that needs work _P-high High priority _P-V1 Priority: slated for V1 release
Milestone

Comments

@hdevalence
Copy link
Member

Is your feature request related to a problem? Please describe.

The ActionList introduced in #4319 represents an in-progress list of actions that will be turned into a TransactionPlan, and provides helper methods for balancing the value of those actions, making it the core of a planner implementation. It should be shared between both planners.

Describe the solution you'd like

Remove the current view/src/planner/action_list.rs, redistributing its contents:

  • the prioritize_and_filter_spendable_notes function should move up into the parent module of the view crate
  • everything else should move into transaction/src/action_list.rs and be exposed as an action_list submodule of the transaction crate

Then update the existing code to match the changed imports.

Additional context

This allows the web code to use the same ActionList.

@github-project-automation github-project-automation bot moved this to Backlog in Penumbra May 6, 2024
@github-actions github-actions bot added the needs-refinement unclear, incomplete, or stub issue that needs work label May 6, 2024
@hdevalence
Copy link
Member Author

hdevalence commented May 6, 2024

@TalDerei the call to canonicalize the action ordering would naturally fit at the end of the ActionList::into_plan method xref #3467

@TalDerei
Copy link
Collaborator

TalDerei commented May 6, 2024

@TalDerei the call to canonicalize the action ordering would naturally fit at the end of the ActionList::into_plan method xref #3467

agreed, I originally added it right after the ActionList::into_plan call in that PR, but I moved it into the method.

@hdevalence
Copy link
Member Author

Sounds great. If we do this part, the next time the web repo updates its rust deps, we'll be able to commonize the fee handling behavior between the two planner implementations.

@TalDerei
Copy link
Collaborator

TalDerei commented May 7, 2024

@hdevalence this needs to be prioritized to fix swap-claim bug identified in penumbra-zone/web#1054. Then we can propagate the changes in penumbra-zone/web#876.

@TalDerei TalDerei added C-design Category: work on the design of Penumbra _P-V1 Priority: slated for V1 release _P-high High priority labels May 7, 2024
@hdevalence
Copy link
Member Author

sounds great, want to take it on?

@TalDerei TalDerei self-assigned this May 7, 2024
@TalDerei
Copy link
Collaborator

TalDerei commented May 7, 2024

@aubrika can you add this to current sprint?

@aubrika aubrika moved this from Backlog to In progress in Penumbra May 7, 2024
@aubrika aubrika added this to the Sprint 6 milestone May 7, 2024
TalDerei added a commit that referenced this issue May 7, 2024
## Describe your changes
Reorganize the `ActionList` struct, enabling the web code to use the
same structure when updating its rust dependencies.
## Issue ticket number and link
References #4332

## Checklist before requesting a review

- [x] If this code contains consensus-breaking changes, I have added the
"consensus-breaking" label. Otherwise, I declare my belief that there
are not consensus-breaking changes, for the following reason:
@TalDerei TalDerei closed this as completed May 7, 2024
@github-project-automation github-project-automation bot moved this from In progress to Done in Penumbra May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-design Category: work on the design of Penumbra needs-refinement unclear, incomplete, or stub issue that needs work _P-high High priority _P-V1 Priority: slated for V1 release
Projects
Archived in project
Development

No branches or pull requests

3 participants