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

auction: add views to the ActionView definition #4295

Merged
merged 7 commits into from
May 1, 2024
Merged

Conversation

erwanor
Copy link
Member

@erwanor erwanor commented Apr 30, 2024

Describe your changes

This PR adds ActionDutchAuctionScheduleView and ActionDutchAuctionWithdrawView to the ActionView message, it also replaces the IsAction perspective impls with concrete implementation.

Populating a view for the withdraw action without extending the TxP structure has proven squirrelly, and it will require a follow-up, although the data is "here" in the view server so I don't think it will be too complicated.

Issue ticket number and link

Part of #4219

Checklist before requesting a review

  • 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:

    Breaking changes are made to ActionView: the action_dutch_auction_schedule and action_dutch_auction_withdraw fields have had their types changes from ActionDutchAuctionSchedule and ActionDutchAuctionWithdraw to ActionDutchAuctionScheduleView and ActionDutchAuctionWithdrawView, respectively.

@erwanor erwanor force-pushed the erwan/txp_txv_da_impls branch from 6d9295f to 2a81ac8 Compare May 1, 2024 00:01
component.auction.v1alpha1.ActionDutchAuctionEnd action_dutch_auction_end = 54;
component.auction.v1alpha1.ActionDutchAuctionWithdraw action_dutch_auction_withdraw = 55;
component.auction.v1alpha1.ActionDutchAuctionWithdrawView action_dutch_auction_withdraw = 55;
Copy link
Member Author

Choose a reason for hiding this comment

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

@jessepinho oversight on our end, but this should fix it.

@erwanor erwanor self-assigned this May 1, 2024
@erwanor erwanor added protobuf-breaking-changes Makes breaking changes to the protobuf definitions. A-auction Area: Relates to the auction component labels May 1, 2024
@erwanor erwanor marked this pull request as ready for review May 1, 2024 01:39
@erwanor erwanor changed the title auction: use view messages in ActionViews auction: add views to the ActionView definition May 1, 2024
@conorsch
Copy link
Contributor

conorsch commented May 1, 2024

Does this break protos that were released, or just what's on main?

@erwanor
Copy link
Member Author

erwanor commented May 1, 2024

we publish to the BSR on main right? the blast damage should only be a branch in the web repo though

@cratelyn
Copy link
Contributor

cratelyn commented May 1, 2024

checking my understanding: this is not a consensus-breaking change because it only performs breaking changes to ActionView, rather than Action.

Copy link
Contributor

@cratelyn cratelyn left a comment

Choose a reason for hiding this comment

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

this looks good to me! 🔨

@erwanor
Copy link
Member Author

erwanor commented May 1, 2024

@cratelyn that's right, however this would be a pretty bad client breaking change

@erwanor
Copy link
Member Author

erwanor commented May 1, 2024

Let's break some protos, shall we 😎

@erwanor erwanor merged commit a500f2c into main May 1, 2024
12 of 13 checks passed
@erwanor erwanor deleted the erwan/txp_txv_da_impls branch May 1, 2024 17:21
@cratelyn cratelyn added this to the Sprint 5 milestone May 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-auction Area: Relates to the auction component protobuf-breaking-changes Makes breaking changes to the protobuf definitions.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants