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

Added end/withdraw all auctions button #1630

Conversation

Valentine1898
Copy link
Contributor

@Valentine1898 Valentine1898 commented Aug 2, 2024

Demo:

Screen.Recording.2024-08-02.at.21.51.44.mov

The limit of max 48 auctions in one transaction is set, but I still have the bug prax-wallet/prax#130 , reviewers, please let me know if you have the same.

Copy link

changeset-bot bot commented Aug 2, 2024

🦋 Changeset detected

Latest commit: d21513b

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
minifront Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Valentine1898 Valentine1898 changed the title WIP: added end/withdraw all auctions button Added end/withdraw all auctions button Aug 2, 2024
// Chain has a transaction size limit, so we can add at most a batch of 48 auctions in a single transaction
// see https://github.com/penumbra-zone/web/issues/1166#issuecomment-2263550249
filterWithLimit(data, a => a.localSeqNum === 0n, 48),
// TODO Should use the index of the selected account after the account selector for the auction is implemented
Copy link
Contributor Author

@Valentine1898 Valentine1898 Aug 2, 2024

Choose a reason for hiding this comment

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

Currently, we can end/withdraw all only for Account 0
see #1166 (comment)

I will open a separate issue for this to add an account selector to the auction list

UPD: I've changed the approach a bit, now users can end and withdraw auctions for all accounts, but auctions from different accounts will end or withdraw with different batches

@Valentine1898 Valentine1898 requested review from jessepinho, a team and hdevalence August 2, 2024 19:05
Comment on lines 27 to 31
export const assembleAuctionBatch = (
auctions: AuctionInfo[],
filteredSeqNumber: bigint,
batchLimit: number,
): AuctionsBatch => {
Copy link
Contributor Author

@Valentine1898 Valentine1898 Aug 5, 2024

Choose a reason for hiding this comment

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

This is a temporary solution that allows you to close all auctions on accounts with different batches
If a user has auctions on account 0 and account 1, the first transaction will end all auctions on account 0, and the second transaction will end all auctions on account 1

see: #1166 (comment)

Copy link
Contributor

@jessepinho jessepinho left a comment

Choose a reason for hiding this comment

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

The logic looks good to me, and could be merged as-is. However, there is a lot of repetition in the ClaimAllButton (which I'd also rename to EndOrWithdrawAllButton). If you could DRY that up, that'd be great — but feel free to merge this first to get this in ASAP.

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.

Only watched UI video but looks good to me

@Valentine1898 Valentine1898 merged commit 6094b6b into main Aug 7, 2024
6 checks passed
@Valentine1898 Valentine1898 deleted the 1166-add-a-claim-all-button-to-the-auctions-list-to-claim-all-unclaimed-reserves-of-ended-auctions branch August 7, 2024 13: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.

Add a "claim all" button to the auctions list to claim all unclaimed reserves of ended auctions
3 participants