-
Notifications
You must be signed in to change notification settings - Fork 38
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(sdk): [NET-1316] Use StreamRegistry
v5 permission methods
#2780
Merged
Conversation
This file contains 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
…and StreamrClient#getAddress
teogeb
force-pushed
the
sdk-StreamRegistry-v5-permission-methods
branch
from
October 2, 2024 11:37
f50dd25
to
5ddee47
Compare
teogeb
force-pushed
the
sdk-StreamRegistry-v5-permission-methods
branch
from
October 2, 2024 13:00
5ddee47
to
39d0bd7
Compare
teogeb
force-pushed
the
sdk-StreamRegistry-v5-permission-methods
branch
from
October 2, 2024 21:26
e8050e1
to
31eec00
Compare
teogeb
added a commit
that referenced
this pull request
Nov 1, 2024
## Summary User id items are stored as arbitrary length `string` instead of `EthereumAddress`. Renamed `getAddess()` in `StreamrClient` to `getUserId()`. **This is a breaking change.** This is a preparation PR for #2780. This PR doesn't doesn't contain any changes to contract / The Graph interaction and therefore any user ids which are not exactly 20 bytes long don't work yet. (We should merge this PR to `main` just before that PR, or implement the contract interactions in a PR which has this PR as a base branch.) ## Changes The `UserID` helper type if now a a separate `BrandedString`. Additionally there is a new helper type `UserIDRaw`, and helper functions `toUserId` and `toUserIdRaw`. There is also the `HexString` helper type which is an alias to `string`. In the API all user id inputs and outputs are `HexString` values. This is not a breaking change as earlier the inputs and outputs were either `string` or `EthereumAddress` values. (The `EthereumAddress` is a branded string, and it was used only in outputs, i.e. in the API all of these were just `string` values.) In `Message`: - field type of `publisherId` In `StreamrClient`: - return type of `getUserId()`, - field type of `UserPermissionQuery#user` - it is parameter type in `hasPermission()` - field type of `UserPermissionAssignment#user` - it is parameter/return type in `getPermissions()`, `grantPermissions()`, `revokePermissions()` and `setPermissions()` - return type of `getStreamPublishers()` and `getStreamSubscribers()` - parametrer type of `isStreamPublisher()`, `isStreamSubscriber()` - field type of `SearchStreamsPermissionFilter#user` - it is a parameter type in `searchStreams()` - changed type of `ResendFromOptions#publisherId` and `ResendRangeOptions#publisherId` - these are parameter types in `resend()` - parameter type of `addEncryptionKey()` In `Stream`: - corresponding changes in `hasPermission()`, `getPermissions()`, `grantPermissions()`, `revokePermissions()` and `setPermissions()` ### Internal In `StreamMessage` the `publisherId` field type is `UserID` (i.e. the branded string) as we don't consider that class to be a part part of the public API. Also the related user id types in `MessageID`/`GroupKeyRequest`/`GroupKeyResponse` are `UserID` strings. ## Future improvements - Change all `EthereumAddress` outputs to `HexString` outputs (e.g. `getStorageNodes()`) to unify the API - Could rename `user` field of `UserPermissionAssignment#user` and `SearchStreamsPermissionFilter#user` to `userId` to unify the API (we have typically the ID suffix in these, e.g. `streamId` and `publisherId`) - Could refactor some test so that we use `randomUserId` instead of `fastWallet` / `ether`'s `Wallet`
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
NOTE: Do not make releases until we've deployed
StreamRegistry
v5 to production.Summary
Updated the sdk to use
StreamRegistry
v5 instead of v4. In v5 there are*forUserId
-permission methods which support arbitrary length user ids.The API is same for all permission types, but longer/shorter IDs is can be used only for publish and subscribe permissions. as there is no contract-level support for other types (edit, grant, delete).
Changes
Replaced usage:
hasPermission
,hasDirectPermission
->getPermissionsForUserId
,getDirectPermissionsForUserId
hasPublicPermission
contract call to check public access, no changes in that logicgrantPermission
->grantPermissionForUserId
revokePermission
->revokePermissionForUserId
setPermissionsMultipleStreams
->setMultipleStreamPermissionsForUserIds
Also changed The Graph queries to use
userId
instead ofuserAddress
.Refactoring
Refactoring the internal call flow in
StreamRegistry
by introducing a helper methodisStreamPublisherOrSubscriber_nonCached
.Tests
Changed
randomUserId
to return random-length user ids.