Update StreamRegistry.sol#4858
Conversation
The implementation uses calldata parameters but the interface specifies memory. This will cause the contract to fail the interface check so I have changed it from calldata to memory.
syed-ghufran-hassan
requested review from
giuseppecrj,
jterzis,
pfives and
shuhuiluo
as code owners
|
@syed-ghufran-hassan is attempting to deploy a commit to the River Build Team Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol:
- Around line 32-37: Change the external function signatures to use calldata for
array/bytes params: update IStreamRegistry.allocateStream to declare nodes and
genesisMiniblock as calldata and then update StreamRegistry.allocateStream to
match (nodes and genesisMiniblock parameters as calldata) so external calls
avoid unnecessary memory copies; also reconcile the related addStream mismatch
by making the interface and implementation signatures consistent (use Stream
calldata in the interface or change implementation to Stream memory) and adjust
any callers accordingly.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol
🧰 Additional context used
📓 Path-based instructions (3)
packages/contracts/**/*.sol
📄 CodeRabbit inference engine (packages/contracts/.cursor/rules/design-philosophy.mdc)
packages/contracts/**/*.sol: Minimize complexity in software structure, prioritizing obviousness and ease of modification in Solidity contracts and facets
Invest time in design upfront rather than quick tactical fixes; prioritize long-term structure over short-term convenience when adding new facets or features
Design code to be obvious: developers should quickly understand existing code and confidently make changes without requiring multiple file traces and dependency navigation
Minimize coupling between modules; use events and interfaces to decouple components so changes in one facet rarely require changes in others
Name entities precisely and document non-obvious behavior; if a function's purpose isn't clear from its signature and name, the design needs improvement
Avoid change amplification: small logical changes should not require modifying many files; high coupling indicates architecture problems
Reduce cognitive load by making facet interfaces self-documenting and isolating storage layouts so developers don't need extensive context to complete tasks
Make it obvious which code must be modified for a given change through clear module boundaries and interfaces to prevent hidden dependencies
Strive for deep modules in Solidity: provide powerful functionality through simple interfaces while hiding complex implementation details
Design facet interfaces to be minimal and clear, hiding internal complexity such as cross-chain calls, rule evaluation, and caching logic
Design facets to be somewhat general-purpose; separate specialized logic from general mechanisms to maximize code reusability
Implement information hiding to ensure each facet has isolated storage slots and facets communicate through libraries, abstracts, or well-defined interfaces rather than shared knowledge
Do not expose storage layout details of one facet to another; each facet must have isolated storage and communicate through interfaces
Avoid temporal decomposition: do not structure code solely ba...
Files:
packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol
packages/contracts/src/**/*.sol
📄 CodeRabbit inference engine (packages/contracts/AGENTS.md)
Organize contract code with separate Storage library, Base contract with internal logic, and Facet contract with external interface
Files:
packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol
**/*.{ts,tsx,js,jsx,yaml,yml,sol,json,md}
📄 CodeRabbit inference engine (AGENTS.md)
All non-Go files must pass Prettier formatting, run bun run prettier:fix to automatically format TypeScript, JavaScript, YAML, Solidity, and other supported files
Files:
packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol
🧠 Learnings (4)
📓 Common learnings
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: packages/contracts/AGENTS.md:0-0
Timestamp: 2025-12-06T08:15:28.770Z
Learning: Applies to packages/contracts/**/*.sol : Use `calldata` for external function parameters that are arrays or strings instead of `memory`
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: packages/contracts/.cursor/rules/contracts.mdc:0-0
Timestamp: 2026-01-08T23:23:01.042Z
Learning: Applies to packages/contracts/**/*.sol : Use calldata in external function argument definitions when possible
Learnt from: shuhuiluo
Repo: towns-protocol/towns PR: 3926
File: packages/contracts/src/factory/facets/feature/FeatureManagerFacet.sol:51-58
Timestamp: 2025-08-22T07:50:18.536Z
Learning: In the towns-protocol contracts, the memory optimization pattern `mstore(0x40, result)` where `result` is a named return variable is extensively used across the codebase in getter functions. When a named return variable is declared, Solidity pre-allocates memory for it, but if the variable gets assigned from a function call that returns a struct, new memory is allocated for the actual data, making the pre-allocated space unused. Reclaiming this space by resetting the free memory pointer is a well-documented gas optimization technique used consistently in files like StreamRegistry.sol, RewardsDistributionV2.sol, MainnetDelegationBase.sol, ReviewFacet.sol, and DropFacet.sol with detailed explanatory comments.
Learnt from: shuhuiluo
Repo: towns-protocol/towns PR: 3926
File: packages/contracts/src/factory/facets/feature/FeatureManagerFacet.sol:51-58
Timestamp: 2025-08-22T07:50:18.536Z
Learning: In the towns-protocol contracts, the memory optimization pattern `mstore(0x40, result)` where `result` is a named return variable is extensively used across the codebase in getter functions. When a named return variable is declared, Solidity pre-allocates memory for it, but if the variable gets assigned from a function call that returns a struct, new memory is allocated for the actual data, making the pre-allocated space unused. Reclaiming this space by resetting the free memory pointer is a well-documented gas optimization technique used consistently in files like StreamRegistry.sol, RewardsDistributionV2.sol, MainnetDelegationBase.sol, ReviewFacet.sol, and DropFacet.sol with detailed explanatory comments.
📚 Learning: 2025-12-06T08:15:28.770Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: packages/contracts/AGENTS.md:0-0
Timestamp: 2025-12-06T08:15:28.770Z
Learning: Applies to packages/contracts/**/*.sol : Use `calldata` for external function parameters that are arrays or strings instead of `memory`
Applied to files:
packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol
📚 Learning: 2025-12-06T08:16:19.499Z
Learnt from: CR
Repo: towns-protocol/towns PR: 0
File: protocol/AGENTS.md:0-0
Timestamp: 2025-12-06T08:16:19.499Z
Learning: Applies to protocol/**/*.proto : Use `bytes` for Ethereum addresses (20 bytes) and stream IDs (32 bytes) in protobuf messages
Applied to files:
packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol
📚 Learning: 2025-12-19T20:33:59.100Z
Learnt from: shuhuiluo
Repo: towns-protocol/towns PR: 4717
File: packages/contracts/test/spaces/membership/unit/MembershipCurrencyMigration.t.sol:2-2
Timestamp: 2025-12-19T20:33:59.100Z
Learning: In Foundry projects, the compiler version is governed by foundry.toml; file-level pragmas only need to be compatible. Do not require exact version matches. Ensure pragma solidity in .sol files is compatible with the version configured in foundry.toml and avoid mismatches that would cause compilation issues.
Applied to files:
packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol
| function allocateStream( | ||
| bytes32 streamId, | ||
| address[] calldata nodes, | ||
| address[] memory nodes, | ||
| bytes32 genesisMiniblockHash, | ||
| bytes calldata genesisMiniblock | ||
| bytes memory genesisMiniblock | ||
| ) external onlyNode(msg.sender) onlyStreamNotExists(streamId) onlyRegisteredNodes(nodes) { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check the StreamRegistry.sol file around lines 32-37
cat -n packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol | sed -n '30,90p'Repository: towns-protocol/towns
Length of output: 2641
🏁 Script executed:
#!/bin/bash
# Look for IStreamRegistry interface definition
find packages/contracts -name "*IStreamRegistry*" -o -name "*StreamRegistry*" | grep -i interfaceRepository: towns-protocol/towns
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check other function signatures in StreamRegistry.sol to see parameter patterns
grep -n "external\|public" packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol | head -20Repository: towns-protocol/towns
Length of output: 1026
🏁 Script executed:
#!/bin/bash
# Search for IStreamRegistry interface definition
find packages/contracts -type f -name "*.sol" | xargs grep -l "interface IStreamRegistry"Repository: towns-protocol/towns
Length of output: 135
🏁 Script executed:
#!/bin/bash
# Also check if there's an interface in the same directory or common paths
find packages/contracts/src/river/registry -type f -name "*.sol" | sortRepository: towns-protocol/towns
Length of output: 764
🏁 Script executed:
#!/bin/bash
# Check the IStreamRegistry interface definition for allocateStream
cat packages/contracts/src/river/registry/facets/stream/IStreamRegistry.solRepository: towns-protocol/towns
Length of output: 7072
Interface should specify calldata for array/bytes parameters per project guidelines.
Both IStreamRegistry.allocateStream and StreamRegistry.allocateStream use memory for the nodes and genesisMiniblock parameters. Project guidelines and learnings require calldata for external function array and bytes parameters—this saves gas by avoiding memory allocation.
Update IStreamRegistry to use calldata, then update the implementation accordingly. Note: addStream has a related issue where the interface specifies Stream memory but the implementation uses Stream calldata.
🤖 Prompt for AI Agents
In @packages/contracts/src/river/registry/facets/stream/StreamRegistry.sol
around lines 32 - 37, Change the external function signatures to use calldata
for array/bytes params: update IStreamRegistry.allocateStream to declare nodes
and genesisMiniblock as calldata and then update StreamRegistry.allocateStream
to match (nodes and genesisMiniblock parameters as calldata) so external calls
avoid unnecessary memory copies; also reconcile the related addStream mismatch
by making the interface and implementation signatures consistent (use Stream
calldata in the interface or change implementation to Stream memory) and adjust
any callers accordingly.
|
Obviously the contracts compile. Change memory to calldata in the interface instead. |
2 participants
The implementation uses calldata parameters but the interface specifies memory. This will cause the contract to fail the interface check so I have changed it from calldata to memory.
Description
Updated function parameters from calldata to memory for interface compliance.
Changes
Checklist
Changeset
Did you modify any published npm packages?
bun changesetand described my changes (usepatchfor most changes)/core, docs, CI, or private packages