feat(cat-gateway): Assets endpoint stake addr lookup w rbac token #2488
Conversation
|
✅ Test Report | |
cong-or
requested review from
kukkok3,
stevenj,
Mr-Leshiy,
saibatizoku,
bkioshn and
stanislav-tkach
as code owners
| let params = GetStakeAddressByCatIDParams::new(token.catalyst_id().clone().into()); | ||
| let mut results = GetStakeAddressByCatIDQuery::execute(&session, params).await?; | ||
|
|
||
| results |
There was a problem hiding this comment.
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-tkach
There was a problem hiding this comment.
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.
It is label as RC2 so I think so, would be okay to perform a build_reg_chain for now
There was a problem hiding this comment.
so the (cat_id,stake_addr) in catalyst_id_for_stake_address table is not the latest?
There was a problem hiding this comment.
It is kind of the latest, but it can lie in several cases. For example:
- A root registration with
Astake address andCID_1Catalyst ID. - A role 0 update that changes a stake address from
AtoB.
After processing these two registrations we would have two entries in the table: [(A, CID_1), (B, CID_1)]. A query with the A address returns CID_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.
@stanislav-tkach does this imply to the inverse also e.g here
There was a problem hiding this comment.
It is label as RC2 so I think so, would be okay to perform a
build_reg_chainfor now
@bkioshn do we have any examples of this?
There was a problem hiding this comment.
@stanislav-tkach does this imply to the inverse also e.g here
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 root registration with
Astake address andCID_1Catalyst ID. - Another root registration with
Astake address andCID_2Catalyst 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.
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.
There was a problem hiding this comment.
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
|
I actually think this PR is mostly fine. |
issue 2393
Replace path parameter to a query parameter for stake address which allows optional stake addr lookup with rbac token, if present.
temporary version to avoid FE breaking changes.
/v2/cardano/assets/