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 select tokens logic #1138

Merged
merged 16 commits into from
Jan 14, 2025
Merged

Conversation

findolor
Copy link
Collaborator

@findolor findolor commented Jan 10, 2025

Motivation

See issue: #1135

We needed to change how select tokens work in our GUI bindings: #1148 and #1149. With this change, we needed to make token field in Gui and Order structs optional, enabling yaml parsing without parsing tokens.

However we still need to have tokens during calldata generation so there are errors that prevent missing tokens during these operations.

Solution

Token Struct

  • Added methods to add and remove token record to the yaml string

Select Tokens

  • This field can now be used to directly manipulate our yaml string. It can be used to add and remove tokens
  • The tokens that are listed in select tokens do not have to be in yaml from the start
  • If a token is referenced in order but not present in the select token list, this throws an error

Context

  • Added select tokens to the context so that we can check our tokens properly in order struct

Checks

By submitting this for review, I'm confirming I've done the following:

  • made this PR as small as possible
  • unit-tested any new functionality
  • linked any relevant issues or PRs
  • included screenshots (if this involves a front-end change)

@findolor findolor requested a review from hardyjosh January 10, 2025 20:12
@findolor findolor self-assigned this Jan 10, 2025
@findolor findolor added the rust Related to rust crates label Jan 13, 2025
@hardyjosh hardyjosh enabled auto-merge January 14, 2025 15:25
@hardyjosh hardyjosh mentioned this pull request Jan 14, 2025
4 tasks
@hardyjosh hardyjosh merged commit 468b9a8 into main Jan 14, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rust Related to rust crates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants