chore: auto-register data provider on first stream creation#1385
Conversation
Time Submission Status
You can submit time with the command. Example: See available commands to help comply with our Guidelines. |
📝 WalkthroughWalkthroughData providers are now auto-created when missing during stream creation via an upsert pattern. Both production and development migration versions implement permissionless onboarding. Stream and taxonomy fee tests are updated to reflect the new universal per-stream fee structure applied regardless of role. ChangesPermissionless provider onboarding and fee updates
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
@holdex pr submit-time 4h |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/streams/stream_creation_fee_test.go (1)
390-410: ⚡ Quick winAssert the onboarding side effect directly.
This proves both calls succeed and both fees are charged, but it never checks that the first call created exactly one
data_providersrow for the wallet and that the second call reused it. A small count query here would make the test validate the PR’s actual contract instead of inferring it from “no error”.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/streams/stream_creation_fee_test.go` around lines 390 - 410, The test only infers data provider registration by balance changes; add an explicit assertion that the data_providers row count for the wallet is exactly 1 after the first create and remains 1 after the second create to verify the ON CONFLICT reuse path. After calling createStream(...) the first time, run a count query against the data_providers table (filter by the wallet address used in createStream) and require it equals 1; after the second createStream(...) run the same count and require it still equals 1. Use the same test helpers/DB handle available in the test harness to perform these queries so the assertions live alongside the existing getBalance(...) checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/migrations/001-common-actions.sql`:
- Around line 145-153: The INSERT into data_providers uses COALESCE((SELECT
MAX(id) FROM data_providers), 0) + 1 for id while using ON CONFLICT (address) DO
NOTHING, which still allows primary-key collisions to abort create_streams;
switch id allocation to a collision-free mechanism (e.g., use the table
sequence/identity instead of MAX(id)+1). Concretely, update the schema to give
data_providers an IDENTITY/sequence or ensure a sequence like
data_providers_id_seq exists and change the INSERT in the data_providers
insertion block to use DEFAULT or nextval('data_providers_id_seq') for id,
keeping ON CONFLICT (address) DO NOTHING so address conflicts are safely ignored
and create_streams no longer fails due to PK collisions.
---
Nitpick comments:
In `@tests/streams/stream_creation_fee_test.go`:
- Around line 390-410: The test only infers data provider registration by
balance changes; add an explicit assertion that the data_providers row count for
the wallet is exactly 1 after the first create and remains 1 after the second
create to verify the ON CONFLICT reuse path. After calling createStream(...) the
first time, run a count query against the data_providers table (filter by the
wallet address used in createStream) and require it equals 1; after the second
createStream(...) run the same count and require it still equals 1. Use the same
test helpers/DB handle available in the test harness to perform these queries so
the assertions live alongside the existing getBalance(...) checks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 634107ec-f2cf-4536-a0ab-4de3692db6d0
📒 Files selected for processing (4)
internal/migrations/001-common-actions.prod.sqlinternal/migrations/001-common-actions.sqltests/streams/stream_creation_fee_test.gotests/streams/taxonomy_fee_test.go
a0e1f5d to
57b5292
Compare
resolves: https://github.com/truflation/website/issues/4002
Summary
data_providersrow when a wallet callscreate_streamsfor the first time, removing the requirement for priorcreate_data_providercall orsystem:network_writerrole grantINSERT ... ON CONFLICT (address) DO NOTHING— idempotent, matches existing pattern inhelper_create_data_providersTest plan
kwil-cli utils parse)TestStreamCreationFees— all 8 tests pass (7 existing + 1 newtestPermissionlessOnboarding)TestTaxonomyFees— all 5 tests pass with corrected funding amountscreate_streamsdirectly without prior registrationSummary by CodeRabbit
New Features
Tests