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

feat(cat-gateway): cardano/rbac/registrations endpoint API definition. #1936

Draft
wants to merge 52 commits into
base: main
Choose a base branch
from

Conversation

Mr-Leshiy
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy commented Feb 28, 2025

Description

Added a new endpoint and get rid of unneeded RBAC endpoints.
Added just an API definition of the cardano/rbac/registrations endpoint.

Also contains #2008

Related Issue(s)

Closes #1923

@Mr-Leshiy Mr-Leshiy added the draft Draft label Feb 28, 2025
@Mr-Leshiy Mr-Leshiy self-assigned this Feb 28, 2025
Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 597/602}$ | ${\color{red}Fail: 0/602}$ |

Copy link
Contributor

github-actions bot commented Mar 4, 2025

Test Report | ${\color{lightgreen}Pass: 642/647}$ | ${\color{red}Fail: 0/647}$ |

cong-or
cong-or previously approved these changes Mar 11, 2025
Mr-Leshiy and others added 4 commits March 12, 2025 13:59
…ntation (#2008)

* RBAC registrations endpoint business logic implementation

* catalyst_id_from_stake function

* UnprocessableContent

* remove attribute

* Update catalyst-gateway/bin/src/service/api/cardano/rbac/registrations_get/mod.rs

Co-authored-by: Alex Pozhylenkov <[email protected]>

* Import

---------

Co-authored-by: Alex Pozhylenkov <[email protected]>
use crate::service::common::{
self,
auth::none_or_api_key::NoneOrApiKey,
Copy link
Collaborator

Choose a reason for hiding this comment

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

The API key was specifically hidden on this endpoint so it would not appear in the API, as was the "ALL" option.
They are internal only options not to be advertised through the public API.

@@ -142,10 +142,10 @@ pub(crate) struct Cip36Details {
pub payment_address: Option<common::types::cardano::cip19_shelley_address::Cip19ShelleyAddress>,
/// If the payment address is a script, then it can not be payed rewards.
#[oai(default)]
pub is_payable: common::types::cardano::boolean::IsPayable,
pub is_payable: common::types::generic::boolean::BooleanFlag,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why remove the specific types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are basically just a boolean flags as BooleanFlag so just removed redundant types.
The description for this field is overlay the default description of the BooleanFlag type.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see your point, but IsCip15 defaults to false, while IsPayable defaults to true, which is why they need to be separate types. This will also affect the OpenAPI spec rendering. Do you have a workaround for handling these default values?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, missed that. Need to take a look is it possible to make a workaround without providing additional types

Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

Response typelooks wrong, probably will cause significant updates to correct it.

#[derive(Debug, Clone, Object)]
#[oai(example = true)]
pub(crate) struct Cip509 {
/// A registration purpose (`UUIDv4`).
Copy link
Collaborator

Choose a reason for hiding this comment

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

This endpoint is supposed to return the complete unified registration as at now, not ll the raw transactions.
This is the wrong response type.

@stanislav-tkach stanislav-tkach self-assigned this Mar 13, 2025
@stanislav-tkach stanislav-tkach marked this pull request as draft March 18, 2025 20:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
do not merge yet PR is not ready to be merged yet review me PR is ready for review
Projects
Status: 🔖 Ready
Development

Successfully merging this pull request may close these issues.

Defined an endpoint interface for RBAC registrations
6 participants