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-voices): cip36 registrations consolidated endpoint #1494

Merged
merged 49 commits into from
Jan 14, 2025
Merged

Conversation

cong-or
Copy link
Contributor

@cong-or cong-or commented Jan 9, 2025

Consolidated endpoint for cip36 registrations.

Example usage:

Single registration with lookup param as vote key:

Single registration bound by time of Unix timestamp:
http://localhost:3030/api/draft/cardano/registration/cip36?lookup=0xa31e0e52dfc72b6bc142c60dd119d46dafe660db7cc3240968c9878497ce9e8e&asat=TIME:1736164751

Single registration bound by time of slot#:
http://localhost:3030/api/draft/cardano/registration/cip36?lookup=0xa31e0e52dfc72b6bc142c60dd119d46dafe660db7cc3240968c9878497ce9e8e&asat=SLOT:11718738

When not bound by time, return latest registration for given vote key.

http://localhost:3030/api/draft/cardano/registration/cip36?lookup=0xa31e0e52dfc72b6bc142c60dd119d46dafe660db7cc3240968c9878497ce9e8e

The same mechanics apply for stake address lookup

http://localhost:3030/api/draft/cardano/registration/cip36?lookup=stake1u9ugz54klca9xqe7nhmdpp23dsp95gkw8snk94gl2rwpdpqcvyze0

.......

ALL registrations: Snapshot

Endpoint requires X-API-Key header with secret token defined in ENV VAR. Contact SRE.

http://localhost:3030/api/draft/cardano/registration/cip36?lookup=ALL

ALL registrations bound by time of slot#:

http://localhost:3030/api/draft/cardano/registration/cip36?lookup=ALL&asat=SLOT:12313103

ALL registrations bound by time of Unix timestamp

http://localhost:3030/api/draft/cardano/registration/cip36?lookup=ALL&asat=TIME:1736164751

cong-or added 30 commits January 4, 2025 21:05
 Stake addresses need to be individually checked to make sure they are still actively
associated with the voting key, and have not been registered to another voting key.
 Stake addresses need to be individually checked to make sure they are still actively
associated with the voting key, and have not been registered to another voting key.
@cong-or cong-or changed the title F14 cip36 feat(cat-voices): cip36 registrations consolidated endpoint Jan 9, 2025
Copy link
Contributor

github-actions bot commented Jan 9, 2025

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

Copy link
Contributor

github-actions bot commented Jan 9, 2025

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

Copy link
Contributor

github-actions bot commented Jan 9, 2025

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

Copy link
Contributor

github-actions bot commented Jan 9, 2025

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

@cong-or cong-or added the review me PR is ready for review label Jan 9, 2025
Copy link
Contributor

@Mr-Leshiy Mr-Leshiy left a comment

Choose a reason for hiding this comment

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

The general issue in this PR is the response types for error cases.
It used only an UnprocessableContent response type for every error case which is not correct.

  • UnprocessableContent should not be returned by us explicitly in most of the cases, it will return during the request arguments parsing/processing.
  • For any database database failure we should call handle_error where in most cases it will just return a 503 response or in some special cases return service_unavailable
  • If something fails because of the incorrect request argument, but it is related to the business logic of the endpoint (e.g. you are getting a stake address which is structurally correct, but during endpoint execution something fails because of it) then it should be BadRequest response, and such response must be specified inside the response enum for such endpoint.

Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

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

Copy link
Contributor

@Mr-Leshiy Mr-Leshiy left a comment

Choose a reason for hiding this comment

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

LGTM

@cong-or cong-or enabled auto-merge (squash) January 14, 2025 10:05
@cong-or cong-or merged commit eb15954 into main Jan 14, 2025
35 checks passed
@cong-or cong-or deleted the f14-cip36 branch January 14, 2025 10:19
Copy link
Contributor

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
F14 review me PR is ready for review
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants