-
Notifications
You must be signed in to change notification settings - Fork 4
Packs multi rip #102
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
base: main
Are you sure you want to change the base?
Packs multi rip #102
Conversation
| } | ||
| } | ||
|
|
||
| function _commitBatch( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to limit the size of commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with a limit as long as we getter/setter. I think 5 - 10 is the range we're looking into right now
| packPrice, | ||
| request.amount | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could be beneficial to add an identifier if we expect one fulfillBatch to be tied to one commitBatch. If not we can probably also use the receiver, and commit block to grab commits to fulfill together.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, what do you think the pros/cons would be? My first hunch was to leave it open so that two unrelated commits could be batch fulfilled in one transaction.
Would it be better processing to handle multi rips as individual plays rather than a batch of independent commits?
I can see that reducing the commit tx overhead, since we could lock the multirip to a single bucket, rather than including redundant bucket data for each commit.
I like this suggestion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
batch of independent would be better. I think having an identifier would just allow the backend to make sure they fulfill bundled commits together so we won't run into an issue where some get fulfilled and some are pending. For LB it's just emitted in the event for tracking not really used on the contract-level.
Independent commits within the batch can still be fulfilled individual if needed and implementaiotn would just decide whether to call bulk ops or single ops
Adds structs for commit/fulfill similar to Lucky Buy. Adds batching of commits/fulfills. Adds an overload for commit/fulfill to support the new struct.