-
Notifications
You must be signed in to change notification settings - Fork 3
feat(permission0): stream funnels #146
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
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughIntroduces an opt-in “funneling” mechanism for stream permissions: new API parameters, storage/event changes, and runtime migration to v8. Updates docs, runtime config/version, and tests. Adds funnel-derived stream ID generation, enable/disable funnel toggling, and emits source/target stream IDs during distribution. Minor IDE config tweak includes tests in cargo check. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Caller
participant Pallet as permission0::Pallet
participant Stream as StreamScope
participant Events as Event
rect rgba(230,240,255,0.5)
note over User,Pallet: Delegate with optional funnel
User->>Pallet: delegate_stream_permission(..., enable_funnel)
Pallet->>Stream: create scope (recipients/streams/accumulation)
alt enable_funnel == true
Stream->>Stream: enable_funnel(permission_id) -> derive StreamId
end
Stream-->>Pallet: scope ready
Pallet-->>User: Ok (PermissionId)
end
rect rgba(240,255,230,0.5)
note over User,Pallet: Update toggling funnel
User->>Pallet: update_stream_permission(..., funnel: Option<bool>)
alt funnel is Some(true/false)
Pallet->>Stream: enable_funnel / disable_funnel
end
Pallet-->>User: Ok
end
rect rgba(255,245,230,0.5)
note over Pallet,Events: Distribution with source/target stream IDs
Pallet->>Stream: execute(permission)
Stream->>Stream: resolve source_stream
alt funnel present
Stream->>Stream: target_stream = derived funnel stream
else
Stream->>Stream: target_stream = source_stream
end
Stream->>Events: emit StreamDistribution{source_stream, target_stream}
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60–90 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
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 |
d5c8605 to
24f3bf2
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/permission0.md (1)
262-404: Fix delegate_stream_permission signature in the docsThe spec still shows the old signature without the new
enable_funnel: boolargument, so readers following this section will try to compile an outdated call. Please add the new parameter (and a short note about its meaning) to keep the docs in sync with the runtime API.
🧹 Nitpick comments (2)
.helix/languages.toml (1)
5-5: Avoid deprecated--allflag in the override command.
cargo check --allemits a deprecation warning on every run. Please switch to--workspace(and, if you need all targets,--all-targets) alongside--teststo keep the command clean.pallets/permission0/src/permission/stream.rs (1)
33-56: Resolve the MaxFunnelsPerPermission wiringWe just introduced
Config::MaxFunnelsPerPermission, but the storage and guard still hard-code a single funnel viaConstU32<1>andensure!(self.funnels.is_empty()). Any runtime that sets the constant to something other than1will still hitTooManyStreamFunnels, so the new knob is effectively dead weight.Please either (a) drop the Config item altogether or (b) wire it through by backing
BoundedVecwithT::MaxFunnelsPerPermissionand comparing againstT::MaxFunnelsPerPermission::get()instead ofis_empty(). That keeps the configuration meaningful and avoids surprising downstream teams.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (13)
.helix/languages.toml(1 hunks)docs/permission0.md(2 hunks)pallets/faucet/tests/faucet.rs(1 hunks)pallets/permission0/api/src/lib.rs(2 hunks)pallets/permission0/src/ext/stream_impl.rs(6 hunks)pallets/permission0/src/lib.rs(8 hunks)pallets/permission0/src/migrations.rs(1 hunks)pallets/permission0/src/permission/stream.rs(8 hunks)pallets/permission0/tests/funnel.rs(1 hunks)pallets/permission0/tests/stream.rs(31 hunks)runtime/src/configs.rs(1 hunks)runtime/src/lib.rs(2 hunks)test-utils/src/lib.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-08-21T22:09:27.601Z
Learnt from: CR
PR: renlabs-dev/torus-substrate#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T22:09:27.601Z
Learning: Applies to {pallets,runtime}/**/*.rs : Use VersionedMigration for storage migrations
Applied to files:
runtime/src/lib.rs
📚 Learning: 2025-08-21T22:09:27.601Z
Learnt from: CR
PR: renlabs-dev/torus-substrate#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-21T22:09:27.601Z
Learning: Applies to {pallets,runtime}/**/*.rs : Increment storage version when changing storage
Applied to files:
pallets/permission0/src/lib.rs
|
can we add event for the funnel creation ? i think it's not there |
|
before enabling or updating a funnel maybe we can check ensure!(matches!(scope.allocation, StreamAllocation::Streams(_)),
Error::<T>::FunnelRequiresStreamsAllocation). so you can not enable funnel on FixedAmount etc |
24f3bf2 to
8e76d32
Compare
|
This patch adds a new operation for stream permissions that happens when distributing the tokens. A "funnel" means that incoming streams for a permission will be emitted as a new, unified stream ID under this permission. The funneling mechanism allows localization and isolation of tokens in complex streaming systems.
I had some trouble doing this as the initial solution was still not clear to me. For some reason, I initially thought I'd have to alter the entire accumulation logic, but it eventually came to me that I'd only have to override when distributing. It took me a while to come to this conclusion.
Closes CHAIN-136.
Summary by CodeRabbit
New Features
Documentation
Tests
Chores