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

fix!: encoding capacity overflow #1249

Merged
merged 49 commits into from
Feb 13, 2024
Merged

Conversation

iqdecay
Copy link
Contributor

@iqdecay iqdecay commented Jan 14, 2024

BREAKING CHANGE:

  • Configurables structs now need to be instantiated through a ::new(encoder_config) or ::default() method.
  • Configurables::with_some_string_config(some_string) methods now return a Result<Configurables> instead of Configurables.
  • Predicates::encode_data now returns a Result<UnresolvedBytes> instead of UnresolvedBytes.
  • PredicateEncoder structs must be instantiated through a ::new(encoder_config) or ::default() method.

@iqdecay iqdecay self-assigned this Jan 14, 2024
Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

Still going through the whole PR, but something caught my attention. But so far, great job!

Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

Quick update on this: This is neat, good job @iqdecay. I'm 70% through it. I should finish this last pass very soon.

Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

I just left some questions/nits. But other than those, LGTM.

@iqdecay iqdecay requested a review from digorithm February 9, 2024 16:57
Copy link
Contributor

@hal3e hal3e left a comment

Choose a reason for hiding this comment

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

Found some small nits that will improve the DevEx

@iqdecay iqdecay requested a review from hal3e February 12, 2024 16:41
hal3e
hal3e previously approved these changes Feb 13, 2024
Copy link
Contributor

@hal3e hal3e left a comment

Choose a reason for hiding this comment

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

Nice work!

MujkicA
MujkicA previously approved these changes Feb 13, 2024
Copy link
Contributor

@segfault-magnet segfault-magnet left a comment

Choose a reason for hiding this comment

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

Turned out great. Safe with no dex damage.

@iqdecay iqdecay dismissed stale reviews from segfault-magnet, MujkicA, and hal3e via f6709bc February 13, 2024 16:44
…les.rs

Co-authored-by: Ahmed Sagdati <37515857+segfault-magnet@users.noreply.github.com>
@iqdecay iqdecay enabled auto-merge (squash) February 13, 2024 17:04
Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

Bring it home. Great work, @iqdecay!

@iqdecay iqdecay merged commit 4cafbdf into master Feb 13, 2024
41 checks passed
@iqdecay iqdecay deleted the iqdecay/fix-encoding-capacity-overflow branch February 13, 2024 17:10
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.

bug: capacity overflow while encoding param types
6 participants