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: remove operations array from the DRP #292

Merged
merged 20 commits into from
Jan 9, 2025
Merged

Conversation

JanLewDev
Copy link
Contributor

@JanLewDev JanLewDev commented Dec 22, 2024

Description

This PR closes #257 and closes #310, removing the need to define the operations array by the DRP devs.

Key features:

  • State changing operations are detected and acted upon, only operations that mutate the state are added to the hashgraph (e.g. if the AddWinsSet holds {1}, add(1) is not creating a new vertex
  • Tests are adapted
  • The check can be bypassed by naming a drp function query_*. By convention such functions are not creating operations
  • Functions that are called from within the drop are also not added to the hashgraph

@JanLewDev JanLewDev marked this pull request as ready for review December 30, 2024 16:43
packages/object/src/index.ts Outdated Show resolved Hide resolved
packages/object/src/index.ts Outdated Show resolved Hide resolved
packages/object/src/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@d-roak d-roak left a comment

Choose a reason for hiding this comment

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

why are we keeping the private functions on the DRPs?

biome.json Show resolved Hide resolved
Comment on lines 20 to 24
query_isWriter: (peerId: string) => boolean;
query_isAdmin: (peerId: string) => boolean;
grant: (senderId: string, peerId: string, publicKey: string) => void;
revoke: (senderId: string, peerId: string) => void;
getPeerKey: (peerId: string) => string | undefined;
query_getPeerKey: (peerId: string) => string | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

put all queries together

packages/object/src/index.ts Outdated Show resolved Hide resolved
Comment on lines +22 to +24
query_isWriter: (peerId: string) => boolean;
query_isAdmin: (peerId: string) => boolean;
query_getPeerKey: (peerId: string) => string | undefined;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
query_isWriter: (peerId: string) => boolean;
query_isAdmin: (peerId: string) => boolean;
query_getPeerKey: (peerId: string) => string | undefined;
query_isAdmin: (peerId: string) => boolean;
query_isWriter: (peerId: string) => boolean;
query_getPeerKey: (peerId: string) => string | undefined;

nitpicking

Copy link
Member

@d-roak d-roak left a comment

Choose a reason for hiding this comment

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

pre-approving

Copy link
Contributor

@trungnotchung trungnotchung left a comment

Choose a reason for hiding this comment

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

lgtm

trungnotchung
trungnotchung approved these changes Jan 9, 2025
Copy link
Contributor

@anhnd350309 anhnd350309 left a comment

Choose a reason for hiding this comment

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

lgtm

@d-roak d-roak merged commit 6c23494 into main Jan 9, 2025
11 of 12 checks passed
@d-roak d-roak deleted the feat/remove-operations-array branch January 13, 2025 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Do not register calls to the DRP that do not change the state as operations Drop Operations[]
4 participants