fix: streamline data provider onboarding via role whitelisting#1137
fix: streamline data provider onboarding via role whitelisting#1137
Conversation
Automatically create data providers when granting system:network_writer role, eliminating manual setup step and improving user experience. - Add helper_create_data_providers batch action for efficient creation - Modify grant_roles to auto-create providers for network_writer grants - Maintain backward compatibility with existing workflows This reduces onboarding friction by consolidating the two-step process (create provider + grant role) into a single role grant operation. resolves: #1136
WalkthroughAdds a private SQL helper to bulk-create data_providers with address validation and idempotent inserts, extends grant_roles to call that helper when granting network_writer, adds role/owner validation helpers, and updates tests to rely on automatic data_provider creation instead of manual setup. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Admin as Grantor
participant DB as grant_roles (SQL)
participant RM as role_members
participant HP as helper_create_data_providers
participant VAL as check_ethereum_address
participant DP as data_providers
Admin->>DB: grant_roles(wallets, roles)
DB->>RM: UPSERT role_members
RM-->>DB: upsert result
alt network_writer included
DB->>HP: helper_create_data_providers(sanitized_wallets)
loop per address
HP->>VAL: check_ethereum_address(addr)
VAL-->>HP: ok / error
end
HP->>DP: INSERT ... UNNEST ... ON CONFLICT DO NOTHING (bulk)
DP-->>HP: inserted / skipped
HP-->>DB: done
end
DB-->>Admin: success
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Bug Report Checklist
|
Time Submission Status
|
|
@pr-time-tracker bug commit 81b3227 && bug author @williamrusdyputra |
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
tests/streams/roles/permission_gates_test.go (2)
44-46: Add an assertion that the data provider was auto-created.Right after setupSystemRoles, assert the data_providers row exists for authorizedWriter to prove the new side effect works end-to-end.
Example (add after Line 46):
// Verify DP auto-created for the granted writer require.NoError(t, setup.RequireDataProviderExists(ctx, platform, authorizedWriter))If setup.RequireDataProviderExists doesn't exist, I can add a tiny helper that queries data_providers and fails if missing.
44-46: Add negative test for invalid wallet on grant.Granting network_writer to an invalid address should error (helper_create_data_providers validates). Add a test to lock this behavior.
Sketch:
t.Run("grant fails for invalid wallet address", func(t *testing.T) { bad := "0xnotanaddress" err := procedure.GrantRoles(ctx, procedure.GrantRolesInput{ Platform: managerPlatform, Owner: "system", RoleName: "network_writer", Wallets: []string{bad}, }) require.Error(t, err) })internal/migrations/013-role-actions.sql (4)
7-10: Clarify docstring to reflect intended scope.State explicitly that it’s invoked only for system:network_writer grants (post-fix above).
Apply:
- * Used by grant_roles when auto-creating data providers during network_writer role assignment. + * Used by grant_roles when auto-creating data providers during system:network_writer role assignment.
19-27: Optional: be explicit about conflict target and add DISTINCT.Small robustness improvement and fewer no-op attempts when the input has duplicates.
Apply (if your dialect supports it):
- FROM UNNEST($addresses) AS t(address) - ON CONFLICT DO NOTHING; + FROM UNNEST($addresses) AS t(address) + ON CONFLICT (address) DO NOTHING;
149-151: Lifecycle note: keep-or-remove data_providers on revoke?Revoke doesn’t touch data_providers, which is fine if providers are durable identities. If the intention is tighter coupling, consider documenting this or adding a cleanup action.
Would you like a follow-up migration/action pair to deactivate (not delete) providers when the role is revoked?
11-27: Add a guard for empty/NULL addresses arrays.Grant already no-ops on empty arrays; mirror that here to avoid a pointless INSERT SELECT.
Apply:
CREATE OR REPLACE ACTION helper_create_data_providers($addresses TEXT[]) PRIVATE { + IF array_length($addresses) = 0 { + RETURN; + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
internal/migrations/013-role-actions.sql(2 hunks)tests/streams/roles/permission_gates_test.go(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: acceptance-test
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
internal/migrations/013-role-actions.sql (1)
152-154: Scope auto-creation to system:network_writer only.Current code provisions providers for any role grant, exceeding the PR intent and surprising other workflows.
Apply:
- -- Auto-create data providers when granting network_writer role - helper_create_data_providers($wallets); + -- Auto-create data providers only for system:network_writer grants + IF $owner = 'system' AND $role_name = 'network_writer' { + helper_create_data_providers($wallets); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
internal/migrations/013-role-actions.sql(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: acceptance-test
🔇 Additional comments (1)
internal/migrations/013-role-actions.sql (1)
12-20: Good: sanitize + validate upfront.Normalizing addresses then validating each keeps the helper safe for direct calls.
Automatically create data providers when granting system:network_writer role, eliminating manual setup step and improving user experience.
This reduces onboarding friction by consolidating the two-step process (create provider + grant role) into a single role grant operation.
resolves: #1136
Note: can't be deployed to production as some file has
UNNESTthat is not ready. But the test is successfull @outerlookSummary by CodeRabbit
New Features
Tests