-
Notifications
You must be signed in to change notification settings - Fork 9
feat(cat-gateway): Assets endpoint stake addr lookup w rbac token #2488
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
Draft
cong-or
wants to merge
16
commits into
main
Choose a base branch
from
lookup-stake-addr-w-rbac-token
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
1ae5299
feat(stake addr lookup w cat id): auth
cong-or f5693e0
feat(stake addr lookup w cat id): auth
cong-or 2bfb361
feat(stake addr lookup w cat id): auth
cong-or c0793a6
feat(stake addr lookup w cat id): auth
cong-or 410bc9e
feat(stake addr lookup w cat id): auth
cong-or a52546e
feat(stake addr lookup w cat id): auth
cong-or 2605241
Merge branch 'main' into lookup-stake-addr-w-rbac-token
cong-or 5faa26c
feat(stake addr lookup w cat id): auth
cong-or 1229618
feat(stake addr lookup w cat id): auth
cong-or 631bc1c
Merge branch 'main' into lookup-stake-addr-w-rbac-token
cong-or 1c47791
feat(staked_ada_get): v2
cong-or b91ec3a
feat(staked_ada_get): v2
cong-or 0f3f688
feat(staked_ada_get): v2
cong-or 4d6394a
Merge branch 'main' into lookup-stake-addr-w-rbac-token
cong-or ae224f6
Merge branch 'main' into lookup-stake-addr-w-rbac-token
cong-or e9dae66
Merge branch 'main' into lookup-stake-addr-w-rbac-token
cong-or File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
7 changes: 7 additions & 0 deletions
7
catalyst-gateway/bin/src/db/index/queries/cql/get_stake_address_from_cat_id.cql
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
SELECT | ||
stake_address | ||
FROM | ||
catalyst_id_for_stake_address | ||
WHERE | ||
catalyst_id = :catalyst_id | ||
ALLOW FILTERING; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
61 changes: 61 additions & 0 deletions
61
catalyst-gateway/bin/src/db/index/queries/rbac/get_stake_address_from_catalyst_id.rs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
//! Get stake address by Catalyst ID. | ||
|
||
use std::sync::Arc; | ||
|
||
use scylla::{ | ||
prepared_statement::PreparedStatement, statement::Consistency, | ||
transport::iterator::TypedRowStream, DeserializeRow, SerializeRow, Session, | ||
}; | ||
use tracing::error; | ||
|
||
use crate::db::{ | ||
index::{ | ||
queries::{PreparedQueries, PreparedSelectQuery}, | ||
session::CassandraSession, | ||
}, | ||
types::{DbCatalystId, DbStakeAddress}, | ||
}; | ||
|
||
/// Get stake address from cat id | ||
const QUERY: &str = include_str!("../cql/get_stake_address_from_cat_id.cql"); | ||
|
||
/// Get stake address by Catalyst ID query params. | ||
#[derive(SerializeRow)] | ||
pub(crate) struct GetStakeAddressByCatIDParams { | ||
/// A Catalyst ID. | ||
pub catalyst_id: DbCatalystId, | ||
} | ||
|
||
impl GetStakeAddressByCatIDParams { | ||
/// Creates a new [`GetStakeAddressByCatIDParams`]. | ||
pub(crate) fn new(catalyst_id: DbCatalystId) -> Self { | ||
Self { catalyst_id } | ||
} | ||
} | ||
|
||
/// Get Catalyst ID by stake address query. | ||
#[derive(Debug, Clone, DeserializeRow)] | ||
pub(crate) struct GetStakeAddressByCatIDQuery { | ||
/// Stake address from Catalyst ID for. | ||
pub(crate) stake_address: DbStakeAddress, | ||
} | ||
|
||
impl GetStakeAddressByCatIDQuery { | ||
/// Prepares a get Catalyst ID by stake address query. | ||
pub(crate) async fn prepare(session: Arc<Session>) -> anyhow::Result<PreparedStatement> { | ||
PreparedQueries::prepare(session, QUERY, Consistency::All, true) | ||
.await | ||
.inspect_err(|e| error!(error=%e, "Failed to prepare get stake address by Catalyst ID")) | ||
} | ||
|
||
/// Executes a get stake address by Catalyst ID query. | ||
pub(crate) async fn execute( | ||
session: &CassandraSession, params: GetStakeAddressByCatIDParams, | ||
) -> anyhow::Result<TypedRowStream<GetStakeAddressByCatIDQuery>> { | ||
session | ||
.execute_iter(PreparedSelectQuery::StakeAddressByCatalystId, params) | ||
.await? | ||
.rows_stream::<GetStakeAddressByCatIDQuery>() | ||
.map_err(Into::into) | ||
} | ||
} |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
6 changes: 3 additions & 3 deletions
6
catalyst-gateway/tests/api_tests/hurl/get_cardano_assets.hurl
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,15 @@ | ||
# Get staked ADA amount: zero assets | ||
GET http://localhost:3030/api/draft/cardano/assets/stake_test1ursne3ndzr4kz8gmhmstu5026erayrnqyj46nqkkfcn0ufss2t7vt | ||
GET http://localhost:3030/api/draft/cardano/assets?stake_address=stake_test1ursne3ndzr4kz8gmhmstu5026erayrnqyj46nqkkfcn0ufss2t7vt | ||
HTTP 200 | ||
{"persistent":{"ada_amount":9809147618,"native_tokens":[],"slot_number":76323283},"volatile":{"ada_amount":0,"native_tokens":[],"slot_number":0}} | ||
|
||
# Get staked ADA amount: single asset | ||
GET http://localhost:3030/api/draft/cardano/assets/stake_test1uq7cnze6az9f8ffjrvkxx4ad77jz088frkhzupxcc7y4x8q5x808s | ||
GET http://localhost:3030/api/draft/cardano/assets?stake_address=stake_test1uq7cnze6az9f8ffjrvkxx4ad77jz088frkhzupxcc7y4x8q5x808s | ||
HTTP 200 | ||
{"persistent":{"ada_amount":8870859858,"native_tokens":[{"amount":3,"asset_name":"GoldRelic","policy_hash":"0x2862c9b33e98096107e2d8b8c072070834db9c91c0d2f3743e75df65"}],"slot_number":76572358},"volatile":{"ada_amount":0,"native_tokens":[],"slot_number":0}} | ||
|
||
# Get staked ADA amount: multiple assets | ||
GET http://localhost:3030/api/draft/cardano/assets/stake_test1ur66dds0pkf3j5tu7py9tqf7savpv7pgc5g3dd74xy0x2vsldf2mx | ||
GET http://localhost:3030/api/draft/cardano/assets?stake_address=stake_test1ur66dds0pkf3j5tu7py9tqf7savpv7pgc5g3dd74xy0x2vsldf2mx | ||
HTTP 200 | ||
[Asserts] | ||
jsonpath "$.persistent.native_tokens" count == 9 |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Not sure whether this will give us a valid
stake_address
that we want 🤔. Might require to build the registration chain and get the latest stake address. What do you think? @stanislav-tkachUh oh!
There was an error while loading. Please reload this page.
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 think that you are right. Anyway, this logic can be greatly simplified when we have the RBAC cache, but we probably cannot wait till that?..
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.
It is label as RC2 so I think so, would be okay to perform a
build_reg_chain
for nowThere 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.
so the (cat_id,stake_addr) in
catalyst_id_for_stake_address
table is not the latest?Uh oh!
There was an error while loading. Please reload this page.
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.
It is kind of the latest, but it can lie in several cases. For example:
A
stake address andCID_1
Catalyst ID.A
toB
.After processing these two registrations we would have two entries in the table:
[(A, CID_1), (B, CID_1)]
. A query with theA
address returnsCID_1
, but that stake address is no longer used by this registration chain, so we should return "not found".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.
@stanislav-tkach does this imply to the inverse also e.g here
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.
@bkioshn do we have any examples of this?
Uh oh!
There was an error while loading. Please reload this page.
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.
Sorry, I was confused. I described the situation with looking up a Catalyst ID by a stake address. Doing the reverse (to search a stake address with a Catalyst ID) would be simply inefficient using the
catalyst_id_for_stake_address
table. But there is an issue there too:A
stake address andCID_1
Catalyst ID.A
stake address andCID_2
Catalyst ID.It is allowed to "override" (or restart) a chain and in this case there once again would be multiple entries and it is impossible to tell which one is correct without building all of the chains. We cannot just use the latest one because it can be incorrect.
Sorry if all that is confusing (it is for me), but that is why we are trying to fix it with a different approach.
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.
This will be resolved with the cache update to rbac registrations, if I am not mistaken.
It may be prudent to pause this and rebase it on that work, rather than try and fix those issues here.
Uh oh!
There was an error while loading. Please reload this page.
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.
Sorry for late reply, @cong-or I have one that you can test
The scenario is
Registration 1: root registration
stake1 =
stake_test1urmunkf4zd332vnucxmsqkddjhhr6a3ww7675phu7pvun0qvaqsqj
catid1 =
preprod.cardano/HM_KavkvqjOjbX9ZeoEZd2mSgsjSJE9FKoVwwbdo04U
Registration 2: root registration
stake2 =
stake_test1uzte0jry3w6rn3j63xhzcyn7fv5akggad66aaffyqsqkzxgm0mhky
catid1 =
preprod.cardano/HM_KavkvqjOjbX9ZeoEZd2mSgsjSJE9FKoVwwbdo04U
Using catid1, you must get the stake address of registration 2
^ The above example will be invalid, I will generate a new one once the cache is ready