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.

Sign up for GitHub

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

Jump to bottom

Fix/rbac indexing #1367

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

Fix/rbac indexing #1367

wants to merge 85 commits into from
+3,123 −2,124

Conversation

stevenj
Copy link
Collaborator

@stevenj stevenj commented Dec 10, 2024 •
edited by stanislav-tkach
Loading

Description

Fixes RBAC indexing issues discovered inside cat-gateway.

Related Issue(s)

Closes #1366, #1759

Description of Changes

Fix RBAC indexing so it works properly and has no boundary conditions.

Breaking Changes

Nothing relies on it yet, so none.

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@stevenj stevenj added the F14 label Dec 10, 2024
@stevenj stevenj added this to the M4: Voting & Delegation milestone Dec 10, 2024
@stevenj stevenj self-assigned this Dec 10, 2024
@stevenj stevenj marked this pull request as draft December 10, 2024 05:52
@minikin minikin added the draft Draft label Dec 30, 2024
@github-actions GitHub Actions
Copy link
Contributor

❌ Test Report | ${\color{lightgreen}Pass: 486/488}$ | ${\color{red}Fail: 2/488}$ |

@github-actions GitHub Actions
Copy link
Contributor

❌ Test Report | ${\color{lightgreen}Pass: 488/495}$ | ${\color{red}Fail: 2/495}$ |

@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 490/495}$ | ${\color{red}Fail: 0/495}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 490/495}$ | ${\color{red}Fail: 0/495}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 490/495}$ | ${\color{red}Fail: 0/495}$ |

@stanislav-tkach stanislav-tkach mentioned this pull request Feb 12, 2025
Open
8 tasks
@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 490/495}$ | ${\color{red}Fail: 0/495}$ |

@github-actions GitHub Actions
Copy link
Contributor

✅ Test Report | ${\color{lightgreen}Pass: 490/495}$ | ${\color{red}Fail: 0/495}$ |

@github-actions GitHub Actions
Copy link
Contributor

❌ Test Report | ${\color{lightgreen}Pass: 489/495}$ | ${\color{red}Fail: 1/495}$ |

bkioshn
bkioshn reviewed Feb 13, 2025
View reviewed changes
catalyst-gateway/bin/src/db/index/schema/cql/catalyst_id_for_stake_addr.cql Outdated Show resolved Hide resolved
catalyst-gateway/bin/src/db/index/block/rbac509/insert_rbac509_invalid.rs Outdated Show resolved Hide resolved
catalyst-gateway/bin/src/db/index/block/txi.rs Show resolved Hide resolved
catalyst-gateway/bin/src/db/index/queries/purge/cql/delete_rbac_registration.cql Outdated Show resolved Hide resolved
catalyst-gateway/bin/src/db/index/queries/purge/cip36_registration.rs Show resolved Hide resolved
catalyst-gateway/bin/src/db/index/queries/purge/cip36_registration_for_vote_key.rs Outdated Show resolved Hide resolved
catalyst-gateway/bin/src/db/index/queries/purge/cip36_registration_invalid.rs Outdated Show resolved Hide resolved
catalyst-gateway/bin/src/db/index/queries/purge/stake_registration.rs Show resolved Hide resolved
catalyst-gateway/bin/src/db/index/schema/cql/txo_assets_by_stake_table.cql
@@ -3,7 +3,7 @@
CREATE TABLE IF NOT EXISTS txo_assets_by_stake (
-- Primary Key Fields
stake_address blob, -- stake address hash (28 bytes stake address hash, zero length if not staked.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we make a clear distinction about this stake_addressand stake_hash In CIP36 stake_address is 32 bytes and here is 28 bytes. Maybe better to rename
stake_address -> stake_public_key since pubkey for Ed25519 32 bytes
stake_hash can remain the same or change to stake_key_hash like in the CIP19 document.

@stevenj

catalyst-gateway/bin/src/db/index/queries/purge/unstaked_txo_assets.rs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stanislav-tkach stanislav-tkach stanislav-tkach left review comments

@Mr-Leshiy Mr-Leshiy Mr-Leshiy left review comments

@bkioshn bkioshn bkioshn left review comments

@cong-or cong-or Awaiting requested review from cong-or cong-or is a code owner

@saibatizoku saibatizoku Awaiting requested review from saibatizoku saibatizoku is a code owner

@minikin minikin Awaiting requested review from minikin

At least 1 approving review is required to merge this pull request.

Assignees

@stevenj stevenj

Labels
draft Draft F14
Projects
Catalyst
Status: 🏗 In progress
Milestone
M4: Voting & Delegation
Development

Successfully merging this pull request may close these issues.

Fix issues with RBAC in cat-gateway
5 participants
@stevenj @stanislav-tkach @Mr-Leshiy @bkioshn @minikin