Proposal: git workflow guidelines prioritizing treceability #114
Conversation
Establishes comprehensive git workflow emphasizing traceability and auditability for blockchain development. Compares merge strategies and recommends merge commits over squash merges to preserve commit history and facilitate code audits.
steinerkelvin
requested review from
DaviPtrs,
saiintbrisson and
devwckd
as code owners
WalkthroughThis change introduces a comprehensive permission and emission delegation system for the blockchain runtime. It adds the new Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Node
participant Permission0 Pallet
participant Emission0 Pallet
participant Governance Pallet
participant Torus0 Pallet
User->>Node: Submit extrinsic (grant_emission_permission)
Node->>Permission0 Pallet: Validate and store permission contract
User->>Node: Submit extrinsic (execute_permission)
Node->>Permission0 Pallet: Check enforcement/revocation, execute emission
Permission0 Pallet->>Emission0 Pallet: Accumulate/distribute emissions
Emission0 Pallet->>Torus0 Pallet: Update stakes, balances
User->>Node: Submit extrinsic (grant_curator_permission)
Node->>Permission0 Pallet: Store curator permission, update indices
User->>Node: Submit extrinsic (toggle_agent_freezing)
Node->>Governance Pallet: Check curator permission via Permission0
Governance Pallet-->>Node: Update freezing state, emit event
Suggested reviewers
Poem
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a series of updates across the Torus pallet and related permission modules. The key changes include:
- Revamped agent registration logic with integrated namespace creation and adjusted error reporting.
- Updates to migrations (v4/v5) and weight calculations to better reflect current benchmarking data.
- Extensive modifications and new test cases in the permission modules (enforcement, curator, fixed, etc.) to enhance permission management and enforcement features.
Reviewed Changes
Copilot reviewed 110 out of 132 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pallets/torus0/src/agent.rs | Updates agent registration validation and integrates namespace creation. |
| pallets/torus0/src/migrations.rs | Introduces updated migration steps (v4 to v5) with reprocessing of agent data. |
| pallets/torus0/src/lib.rs | Refactors call weight usage and updates pallet storage version to v5. |
| pallets/torus0/src/burn.rs | Revises burn adjustment calculations with FixedU128 conversions. |
| pallets/permission0/tests/* | Expands and refines tests for permission enforcement, curator functionality, and fixed permissions. |
| pallets/torus0/api/* and pallets/torus0/rpc/* | Updates API and RPC interfaces to align with recent changes. |
Comments suppressed due to low confidence (3)
pallets/torus0/src/agent.rs:63
- The agent registration now validates using the payer's eligibility via 'can_register_agent' and returns an 'AgentsFrozen' error. Please verify that this error label accurately reflects the intended logic and does not confuse callers expecting a whitelist-related error.
);
pallets/torus0/src/lib.rs:320
- The registration extrinsic now uses T::WeightInfo::register_agent for its weight. Ensure that the corresponding weight benchmarks and estimates accurately reflect the cost in production environments.
#[pallet::weight((T::WeightInfo::register_agent(), DispatchClass::Normal, Pays::Yes))]
pallets/torus0/src/burn.rs:95
- When converting 'next_value' to the underlying Balance via 'into_inner()', please ensure that the conversion is safe and documented, especially regarding potential overflow risks or differences in numeric types.
let new_burn = if next_value >= FixedU128::from_inner(max_burn) {
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 32
🔭 Outside diff range comments (3)
pallets/governance/src/migrations.rs (1)
8-333: Eliminate code duplication between v3 and v5 migrationsThe v3 and v5 migrations contain nearly identical code for migrating proposals. This duplication increases maintenance burden and risk of inconsistencies.
Consider either:
- Removing v5 if it's redundant with v3
- Extracting common migration logic into a shared helper function
- Documenting why both migrations are necessary if they serve different purposes
Example refactor:
mod migration_helpers { pub fn migrate_proposal<T: Config>(old_proposal: old_storage::Proposal<T>) -> crate::proposal::Proposal<T> { // Common migration logic here } }pallets/governance/src/proposal.rs (2)
89-94: Handle transfer failure when returning proposal costThe error from transferring the proposal cost back to the proposer is ignored. If this transfer fails, the proposal will still execute, which could lead to treasury funds being locked.
- let _ = <T as crate::Config>::Currency::transfer( + <T as crate::Config>::Currency::transfer( &crate::DaoTreasuryAddress::<T>::get(), &self.proposer, self.proposal_cost, ExistenceRequirement::AllowDeath, - ); + ) + .map_err(|_| crate::Error::<T>::InternalError)?;
551-561: Replace expect() with proper error handling in loopsUsing
expect()in loops that process external data could lead to panics if constraints are violated. Consider usingsaturating_insertor logging errors instead.for (key, value) in votes_for { - reward_votes_for - .try_insert(key, value) - .expect("this wont exceed u32::MAX"); + // Use saturating behavior or log if capacity is exceeded + if reward_votes_for.try_insert(key, value).is_err() { + error!("Failed to insert vote for proposal {}: exceeded max capacity", proposal_id); + } }Apply the same pattern to the
votes_againstloop.
♻️ Duplicate comments (1)
pallets/governance/api/Cargo.toml (1)
9-10: Consistent lint strategy maintainedMatch with emission0 API; same verification of workspace root applies. See earlier comment.
🧹 Nitpick comments (57)
.gitignore (1)
1-2:*.local.*pattern might ignore more than intendedThe glob will hide any file whose middle segment is
.local.(e.g.config.local.json,prod.local.env, but alsomodule.local.rs).
If you only want to suppress env-specific dot-files, consider narrowing:-*.local.* +# Env / editor overrides +*.local.env +*.local.json +*.local.tomlThis reduces accidental loss of source files placed in
src/…/*.local.rs, which some Rust projects use for feature gating.docs/linear-emission.md (2)
9-12: Minor wording & precision improvements for emission mathCurrent text mixes countable “tokens” with “amount of”, and the
64000 / 10800rate truncates an infinite decimal.
Proposal:-On every new block (target: 8 seconds), a certain amount of tokens is added to the `PendingEmission` -storage value, in a way that 64000 tokens are emitted per day (10800 blocks). The amount is controlled -by a parameter called `BlockEmission`, which currently is `64000 / 10800`. +Every block (≈ 8 s) the system mints **a fixed number of tokens** into `PendingEmission`. +The target is **64 000 tokens per day**, i.e. +`BlockEmission = 64 000 / 10 800 ≈ 5.925 925...` tokens. +The exact rational is stored as a `FixedU128` to avoid rounding drift.Helps readers reproduce the constant and clarifies representation.
31-38: Long sentence obscures validator-selection ruleLines 35-37 pack several conditions in one breath. Splitting improves clarity and mirrors the implementation logic:
-If the total stake is larger than the `MinValidatorStake` value and it has weights set, the weights are -filtered so that weights set on themselves are excluded and the remaining ones only refer to registered -agents/agents deregistered on last epoch and are finally normalized. +An input qualifies for validator selection when: +1. its total stake ≥ `MinValidatorStake`; +2. it possesses at least one non-zero weight. +Self-votes are stripped, and the remaining weights are constrained to agents that are either currently +registered or were deregistered in the previous epoch. +The resulting vector is then normalised.No semantic change—just readability.
pallets/permission0/rpc/Cargo.toml (1)
1-15: Add basic metadata to conform with Cargo package policyThe manifest lacks
license,repository, anddescriptionfields. Most downstream tooling (crates-io, cargo-deny, internal license scanners) expect these to be present, even for in-workspace crates that are not published.[package] name = "pallet-permission0-rpc" version = "0.1.0" edition = "2021" +license = "MIT-0" +description = "RPC interface for the permission0 pallet" +repository = "https://github.com/<org>/torus"Including them keeps all crates in the workspace uniformly compliant and avoids false positives in license/compliance checks.
.github/workflows/build-runtime.yml (1)
40-42: Shell interpolation is fine, but quoting is unnecessary noiseThe inline expression already expands to an empty string or
--features testnet; surrounding it with single quotes inside the expression produces the same output, but the extra quoting is redundant and hurts readability.- cargo build --release --timings --package torus-runtime ${{ startsWith(github.ref, 'refs/tags/runtime/testnet') && '--features testnet' || '' }} + cargo build --release --timings --package torus-runtime ${{ startsWith(github.ref, 'refs/tags/runtime/testnet') && '--features testnet' || '' }}(i.e. drop the inner single quotes)
.github/workflows/build-docs.yml (1)
7-7: Trim trailing whitespaceLines 7 and 20 have trailing spaces, flagged by yamllint. Cleaning them avoids noisy diffs and keeps lint pipelines green.
Also applies to: 20-20
pallets/governance/src/whitelist.rs (2)
5-6: Minor wording tweak for clarity
/// Adds a key to the DAO whitelist, allowing it to register an agent.
Consider “…allowing the key to register an agent.” to keep subject/object consistent.
20-22: Doc string split mid-sentenceThe sentence is broken across two
///lines, yielding an odd line-break in generated docs. Combine for smoother rendering:/// Removes a key from the DAO whitelist, disallowing it from registering /// an agent or de-registering an existing one.pallets/torus0/api/src/api.rs (1)
1-1: Consider addressing the clippy warning.The
clippy::multiple_bound_locationsallow directive suggests there might be duplicate trait bounds. While not critical, consider reviewing if the bounds can be simplified for better maintainability.docs/deploy.md (3)
11-11: Fix preposition usage.- - The .zip file can be found on the `Artifacts` section of the Action that built it. + - The .zip file can be found in the `Artifacts` section of the Action that built it.
13-13: Fix preposition usage.- - The WASM blob to be used on the next steps is inside the downloaded zipped file. + - The WASM blob to be used in the next steps is inside the downloaded zipped file.
22-22: Add missing comma after introductory phrase.-> As of april 9th 2025 the try-runtime cli is broken, install the fork by running `cargo install --git https://github.com/renlabs-dev/try-runtime-cli --locked`. +> As of april 9th 2025, the try-runtime cli is broken, install the fork by running `cargo install --git https://github.com/renlabs-dev/try-runtime-cli --locked`.pallets/torus0/src/burn.rs (1)
95-101: Consider using checked arithmetic for bounds checking.While the current logic is correct, consider using
checked_cmpmethods for more explicit bounds checking instead of comparison operators on FixedU128 values.- let new_burn = if next_value >= FixedU128::from_inner(max_burn) { + let new_burn = if next_value.into_inner() >= max_burn { max_burn - } else if next_value <= FixedU128::from_inner(min_burn) { + } else if next_value.into_inner() <= min_burn { min_burn } else { next_value.into_inner() };pallets/governance/api/src/lib.rs (1)
14-14: Consider return type consistency.The
get_allocators()method returnsimpl Iterator<Item = AccountId>while other methods return concrete types. Consider whether this is the most appropriate return type for external API consumers.If external consumers need to collect or process the iterator multiple times, consider returning
Vec<AccountId>for consistency with typical API patterns:- fn get_allocators() -> impl Iterator<Item = AccountId>; + fn get_allocators() -> Vec<AccountId>;pallets/governance/tests/roles.rs (1)
26-32: Simplify redundant balance check logic.The balance check logic appears redundant. Line 26 asserts
balance >= stake, but then lines 30-32 check ifbalance - stake < 1and add balance if needed. This suggests the assertion might be incorrect or the logic could be simplified.Consider this refactor:
- assert!(get_balance(account) >= stake); - let min_stake: u128 = pallet_torus0::MinAllowedStake::<Test>::get(); if stake >= min_stake { - if get_balance(account) - stake < 1 { + if get_balance(account) < stake + 1 { add_balance(account, 1); }.github/workflows/check.yml (1)
116-117: Consider parameterizing hardcoded S3 configuration.The hardcoded values for region and endpoint domain could be moved to workflow variables for better maintainability.
- REGION: nyc3 - ENDPOINT_DOMAIN: "digitaloceanspaces.com" + REGION: ${{ vars.COV_REGION || 'nyc3' }} + ENDPOINT_DOMAIN: ${{ vars.COV_ENDPOINT_DOMAIN || 'digitaloceanspaces.com' }}pallets/emission0/src/weight_control.rs (1)
87-90: Add documentation for the restriction rationale.Consider expanding the comment to explain why delegation is restricted to allocators at the current network stage.
- // At the current network stage, it only makes sense to delegate weight control - // to allocators. + // At the current network stage, weight control delegation is restricted to allocators + // to ensure proper governance oversight and prevent unauthorized weight manipulation.pallets/permission0/src/migrations.rs (1)
19-35: Consider consolidating duplicate migration code.The
v2andv3migration modules have identical structure with only version numbers differing. Consider creating a generic migration helper to reduce code duplication.+pub struct NoOpMigration<T, const FROM: u32, const TO: u32>(core::marker::PhantomData<T>); + +impl<T: Config, const FROM: u32, const TO: u32> UncheckedOnRuntimeUpgrade for NoOpMigration<T, FROM, TO> { + fn on_runtime_upgrade() -> Weight { + Weight::zero() + } +} pub mod v2 { use polkadot_sdk::{ frame_support::{migrations::VersionedMigration, traits::UncheckedOnRuntimeUpgrade}, sp_weights::Weight, }; use crate::{Config, Pallet}; - pub type Migration<T, W> = VersionedMigration<1, 2, MigrateToV2<T>, Pallet<T>, W>; - pub struct MigrateToV2<T>(core::marker::PhantomData<T>); - - impl<T: Config> UncheckedOnRuntimeUpgrade for MigrateToV2<T> { - fn on_runtime_upgrade() -> Weight { - Weight::zero() - } - } + pub type Migration<T, W> = VersionedMigration<1, 2, super::NoOpMigration<T, 1, 2>, Pallet<T>, W>; } pub mod v3 { use polkadot_sdk::{ frame_support::{migrations::VersionedMigration, traits::UncheckedOnRuntimeUpgrade}, sp_weights::Weight, }; use crate::{Config, Pallet}; - pub type Migration<T, W> = VersionedMigration<2, 3, MigrateToV3<T>, Pallet<T>, W>; - pub struct MigrateToV3<T>(core::marker::PhantomData<T>); - - impl<T: Config> UncheckedOnRuntimeUpgrade for MigrateToV3<T> { - fn on_runtime_upgrade() -> Weight { - Weight::zero() - } - } + pub type Migration<T, W> = VersionedMigration<2, 3, super::NoOpMigration<T, 2, 3>, Pallet<T>, W>; }pallets/permission0/rpc/src/lib.rs (1)
40-47: Enhance error handling granularity.The current error handling uses a generic error code (1) for all runtime execution failures. Consider using different error codes for different failure scenarios to help clients distinguish between error types.
runtime .root_stream_id_for_account(at, account) - .map_err(|err| ErrorObject::owned(1, "Runtime execution failed", Some(err.to_string()))) + .map_err(|err| { + // Use different error codes for different error types + let (code, message) = match err { + sp_api::ApiError::Application(_) => (1001, "Application error"), + sp_api::ApiError::UnknownVersion => (1002, "Unknown API version"), + sp_api::ApiError::FailedToExtractRuntimeVersion => (1003, "Runtime version extraction failed"), + _ => (1000, "Runtime execution failed"), + }; + ErrorObject::owned(code, message, Some(err.to_string())) + })justfile (1)
51-54: Verify benchmark command consistency.The benchmark commands have different formatting styles. Consider standardizing the line breaks and parameter formatting for consistency.
cargo build -r --features runtime-benchmarks - ./target/release/torus-node benchmark pallet --pallet pallet_torus0 --chain dev --extrinsic "*" --steps 50 --repeat 20 --output pallets/torus0/src/weights.rs --template=./.maintain/frame-weight-template.hbs + ./target/release/torus-node benchmark pallet \ + --pallet pallet_torus0 \ + --chain dev \ + --extrinsic "*" \ + --steps 50 \ + --repeat 20 \ + --output pallets/torus0/src/weights.rs \ + --template=./.maintain/frame-weight-template.hbs ./target/release/torus-node benchmark pallet --pallet pallet_governance --chain dev --extrinsic "*" --steps 50 --repeat 20 --output pallets/governance/src/weights.rs --template=./.maintain/frame-weight-template.hbs ./target/release/torus-node benchmark pallet --pallet pallet_emission0 --chain dev --extrinsic "*" --steps 50 --repeat 20 --output pallets/emission0/src/weights.rs --template=./.maintain/frame-weight-template.hbs - ./target/release/torus-node benchmark pallet --pallet pallet_permission0 --chain dev --extrinsic "*" --steps 50 --repeat 20 --output pallets/permission0/src/weights.rs --template=./.maintain/frame-weight-template.hbs + ./target/release/torus-node benchmark pallet \ + --pallet pallet_permission0 \ + --chain dev \ + --extrinsic "*" \ + --steps 50 \ + --repeat 20 \ + --output pallets/permission0/src/weights.rs \ + --template=./.maintain/frame-weight-template.hbspallets/torus0/rpc/src/lib.rs (1)
53-67: Simplify nested error handling.The current nested error handling with two levels of
map_erris complex and could be simplified for better readability.runtime .namespace_path_creation_cost(at, account_id, path) - .map(|res| { - res.map_err(|err| { - ErrorObject::owned( - 1, - "namespace cost calculation failed", - Some(format!("{err:?}")), - ) - }) - }) - .map_err(|err| { - ErrorObject::owned(1, "Runtime execution failed", Some(err.to_string())) - })? + .map_err(|err| { + ErrorObject::owned(1001, "Runtime execution failed", Some(err.to_string())) + })? + .map_err(|err| { + ErrorObject::owned(1002, "Namespace cost calculation failed", Some(format!("{err:?}"))) + })pallets/permission0/tests/curator.rs (1)
90-99: Consider testing edge cases with combined permissions.The current test checks individual permissions, but consider adding a test case for operations requiring multiple permission flags combined with bitwise operations.
for flags in [ CuratorPermissions::APPLICATION_REVIEW, CuratorPermissions::PENALTY_CONTROL, + CuratorPermissions::APPLICATION_REVIEW | CuratorPermissions::PENALTY_CONTROL, ] { assert_err!( ensure_curator(RawOrigin::Signed(key).into(), flags), Error::<Test>::PermissionNotFound ); } + + // Test that combined permissions work when granted + grant_curator_permission(key, CuratorPermissions::APPLICATION_REVIEW | CuratorPermissions::PENALTY_CONTROL, None); + assert_ok!(ensure_curator( + RawOrigin::Signed(key).into(), + CuratorPermissions::APPLICATION_REVIEW | CuratorPermissions::PENALTY_CONTROL + ));docs/development/git-workflow.md (2)
79-84: Useful branch cleanup commandThis cleanup command is practical and helps maintain a tidy local repository. Consider adding a note about using
--dry-runfirst for safety.+# Preview which branches would be deleted: +git branch --merged main | grep -v '\*' | xargs -n 1 echo + +# Actually delete the merged branches: git branch --merged main | grep -v '\*' | xargs -n 1 git branch -d
101-121: Consider implementing commit message conventions nowRather than leaving this as a TODO, consider establishing these conventions immediately to ensure consistency from the start.
Would you like me to create a more detailed commit message convention document or open an issue to track this task?
pallets/torus0/src/benchmarking.rs (1)
14-21: Consider consistent error handling in helper functionThe helper function uses
expectwhich will panic on failure. Since this is for benchmarking, consider whether a more descriptive error message would be helpful.fn register_test_agent<T: Config>( id: &T::AccountId, name: Vec<u8>, url: Vec<u8>, metadata: Vec<u8>, ) { - Pallet::<T>::force_register_agent(id, name, url, metadata).expect("failed to register agent"); + Pallet::<T>::force_register_agent(id, name, url, metadata) + .expect("benchmark setup: failed to register test agent"); }pallets/emission0/tests/weights.rs (1)
26-30: Address the commented TODO section.The commented test code indicates incomplete functionality. Ensure that when weight control is re-enabled, these tests are uncommented and validated.
Do you want me to help track this as a separate issue or provide guidance on when these tests should be re-enabled?
CONTRIBUTING.md (3)
42-43: Minor punctuation improvement needed.Consider adding a colon after "main crates" for better readability.
-Our project is organized into 5 main crates: +Our project is organized into 5 main crates:
56-56: Add missing comma for clarity.The sentence would benefit from a comma for better readability.
-We follow standard Rust coding conventions with some additional requirements specific to blockchain development. +We follow standard Rust coding conventions, with some additional requirements specific to blockchain development.
64-64: Add missing article.The sentence needs "The" before "last thing" for proper grammar.
-Last thing you want is a massive Cargo file with 150 substrate dependencies written out manually. +The last thing you want is a massive Cargo file with 150 substrate dependencies written out manually.pallets/governance/src/benchmarking.rs (1)
27-43: Consider using specific curator permissions for benchmarks.The
curator()helper grants all permissions usingCuratorPermissions::all(), which may be more than necessary for individual benchmarks. For more precise testing, consider granting only the required permissions for each test case.For example, modify the curator helper to accept specific permissions:
-fn curator<T: Config>() -> T::AccountId { +fn curator<T: Config>(permissions: CuratorPermissions) -> T::AccountId { let curator_id: T::AccountId = account("Curator", 10, 10); <<T as crate::Config>::Permission0 as Permission0CuratorApi< T::AccountId, OriginFor<T>, BlockNumberFor<T>, >>::grant_curator_permission( RawOrigin::Root.into(), curator_id.clone(), - CuratorPermissions::all(), + permissions, None, PermissionDuration::Indefinite, RevocationTerms::Irrevocable, ) .unwrap(); curator_id }Then use specific permissions in benchmarks:
- add_to_whitelist(RawOrigin::Signed(curator::<T>()), module_key) + add_to_whitelist(RawOrigin::Signed(curator::<T>(CuratorPermissions::WHITELIST_MANAGE)), module_key)pallets/faucet/src/lib.rs (1)
64-69: Simplify redundant storage version definitionsBoth feature-gated storage version definitions are identical. This can be simplified to a single definition.
- /// Storage version for the pallet - #[cfg(feature = "testnet")] - const STORAGE_VERSION: StorageVersion = StorageVersion::new(0); - - /// Storage version for the pallet (production networks) - #[cfg(not(feature = "testnet"))] + /// Storage version for the pallet const STORAGE_VERSION: StorageVersion = StorageVersion::new(0);pallets/permission0/src/ext/namespace_impl.rs (1)
74-74: Track the ROOT curator TODOThe comment indicates that the parent field will change once a ROOT curator is implemented. This should be tracked as a technical debt item.
Would you like me to create an issue to track the ROOT curator implementation?
docs/pallet-structure.md (3)
69-77: Add language specifier to code blockThe code block should specify the language for proper syntax highlighting.
-``` +```rust // Instead of: let result = (amount * percentage) / 100; // We use: let percentage = FixedU128::from_rational(percentage_value, 100); let result = percentage.mul_floor(amount);--- `134-134`: **Add missing comma for clarity** ```diff -Blockchains require careful handling of storage changes since data persists across runtime upgrades. +Blockchains require careful handling of storage changes, since data persists across runtime upgrades.
222-222: Simplify redundant phraseThe phrase "evolve over time" is redundant; "evolve" already implies change over time.
-By following these patterns and conventions, we create a robust and maintainable blockchain system that can evolve over time while maintaining backward compatibility and performance. +By following these patterns and conventions, we create a robust and maintainable blockchain system that can evolve while maintaining backward compatibility and performance.pallets/permission0/tests/enforcement.rs (1)
1-1: Avoid blanket clippy suppressionInstead of suppressing the
indexing_slicingwarning globally, consider using safer alternatives like.get()or be more specific about which lines need the suppression.Remove the global suppression and use safer indexing:
-#![allow(clippy::indexing_slicing)]For example, on line 94:
- assert_eq!(controllers[0], controller); + assert_eq!(controllers.get(0), Some(&controller));README.md (1)
157-157: Simplify wording for better readability.Consider simplifying the phrase for clarity:
-| Stake | Tokens locked in support of an agent. Higher stakes affect the weig... +| Stake | Tokens locked supporting an agent. Higher stakes affect the weig...pallets/faucet/src/faucet.rs (2)
82-82: Consider making difficulty configurable.The hardcoded difficulty value reduces flexibility for different testnet environments. Consider adding it as a pallet configuration parameter.
- let difficulty: U256 = U256::from(1_000_000); + let difficulty: U256 = U256::from(T::FaucetDifficulty::get());
94-95: Verify the token minting approach aligns with emission controls.Using
deposit_creatingmints tokens out of thin air, bypassing any emission or supply controls. For a testnet faucet this might be acceptable, but ensure this aligns with the broader tokenomics design.docs/namespace.md (1)
68-68: Fix minor grammatical issues.Several sentences would benefit from improved punctuation and article usage.
- Agents think carefully about namespace creation since deposits are locked. + Agents think carefully about namespace creation, since deposits are locked. - The agent can delegate read access to `agent.memory.twitter` to analytics agents while keeping other sources private. + The agent can delegate read access to `agent.memory.twitter` to analytics agents, while keeping other sources private. - The hierarchical structure naturally represents the hardware taxonomy while permissions control access. + The hierarchical structure naturally represents the hardware taxonomy, while permissions control access. - This may be refined in future versions as usage patterns emerge and economic requirements become clearer. + This may be refined in future versions as usage patterns emerge, and economic requirements become clearer. -We will need a anti-spam system to emerge in the near future. +We will need an anti-spam system to emerge in the near future.Also applies to: 121-121, 123-123, 135-135, 149-149
pallets/permission0/src/permission/emission.rs (1)
134-142: Use iterator sum() for better readabilityThe fold operation can be replaced with a more idiomatic sum().
let accumulated = match &emission_scope.allocation { EmissionAllocation::Streams(streams) => streams .keys() .filter_map(|id| { AccumulatedStreamAmounts::<T>::get((&contract.grantor, id, permission_id)) }) - .fold(BalanceOf::<T>::zero(), |acc, e| acc.saturating_add(e)), // The Balance AST does not enforce the Sum trait + .reduce(|acc, e| acc.saturating_add(e)) + .unwrap_or_else(BalanceOf::<T>::zero), EmissionAllocation::FixedAmount(amount) => *amount, };pallets/governance/tests/proposal.rs (2)
401-401: Define constants for magic block numbersThe block number 21,600 appears multiple times without explanation of its significance.
+// 21,600 blocks = ~2 days at 6 second block time +const PROPOSAL_EXECUTION_DELAY: u64 = 21_600; + assert_ok!(pallet_governance::Pallet::<Test>::add_emission_proposal( origin.clone(), Percent::from_parts(20), Percent::from_parts(20), Percent::from_parts(20), vec![b'0'; 64], )); vote(0, 0, true); -step_block(21_600); +step_block(PROPOSAL_EXECUTION_DELAY);Also applies to: 441-441, 459-459
18-30: Extract test constants for better maintainabilityThe default_params function could benefit from test-specific constants.
Consider creating a test configuration module:
mod test_config { pub const DEFAULT_PROPOSAL_METADATA: &[u8] = b"metadata"; pub const MINIMUM_PROPOSAL_STAKE: u128 = 1; pub const TEST_AGENT_NAME: &[u8] = b"agent"; // ... other constants }pallets/permission0/src/lib.rs (2)
482-495: Consider caching total allocated percentagesThis function iterates through all permissions for a stream on every call, which could be expensive.
Consider maintaining a storage item that tracks total allocation per stream:
#[pallet::storage] pub type TotalStreamAllocation<T: Config> = StorageDoubleMap< _, Identity, T::AccountId, Identity, StreamId, Percent, ValueQuery, >;Update this value when permissions are granted/revoked to avoid repeated calculations.
42-42: Document storage migration pathThe storage version is set to 3, indicating previous migrations.
Add documentation about:
- What changed in each storage version
- Migration requirements when upgrading
- Any breaking changes
/// Storage version history: /// - v1: Initial implementation /// - v2: Added enforcement authority support /// - v3: Refactored permission scope structure const STORAGE_VERSION: StorageVersion = StorageVersion::new(3);pallets/torus0/api/src/lib.rs (1)
184-194: Consider iterator-based approach for efficiencyThe
parents()method allocates a vector for all parent paths, which could be inefficient for deeply nested namespaces in a blockchain context where computational resources are metered.Consider providing an iterator-based alternative:
pub fn parents_iter(&self) -> impl Iterator<Item = Self> + '_ { struct ParentsIter<'a> { current: Option<NamespacePath>, _marker: core::marker::PhantomData<&'a ()>, } impl<'a> Iterator for ParentsIter<'a> { type Item = NamespacePath; fn next(&mut self) -> Option<Self::Item> { self.current.as_ref()?.parent().map(|parent| { self.current = Some(parent.clone()); parent }) } } ParentsIter { current: Some(self.clone()), _marker: core::marker::PhantomData, } }pallets/permission0/src/permission.rs (1)
315-359: Optimize auto-execution with single iterationThe function iterates over all permissions twice, which could be inefficient with many permissions. Also, pre-allocating the expired vector with full capacity may waste memory.
Combine both operations in a single iteration:
- let permissions: Vec<_> = Permissions::<T>::iter().collect(); - let mut expired = Vec::with_capacity(permissions.len()); + let mut expired = Vec::new(); - for (permission_id, contract) in Permissions::<T>::iter() { + for (permission_id, contract) in Permissions::<T>::iter() { #[allow(clippy::single_match)] match &contract.scope { PermissionScope::Emission(emission_scope) => { emission::do_auto_distribution( emission_scope, permission_id, current_block, &contract, ); } _ => (), } if contract.is_expired(current_block) { expired.push((permission_id, contract)); } }pallets/emission0/src/lib.rs (1)
101-108: Complex trait bounds for Permission0 integrationThe Permission0 associated type requires multiple trait implementations with numerous generic parameters. While this adds complexity, it appears necessary for the comprehensive permission system integration. Consider documenting the purpose of each generic parameter for implementers.
pallets/torus0/src/agent.rs (3)
81-85: Improve error handling specificity for namespace path validation.The current implementation loses valuable error context when converting namespace path errors. The warning logs the specific error but returns a generic error to the user.
Consider preserving the error details or creating more specific error variants.
171-179: Optimize cooldown check by performing it before mutation.The cooldown check currently happens inside
try_mutate, which acquires a lock. This could be optimized by checking the cooldown before attempting mutation.- crate::Agents::<T>::try_mutate(&agent_key, |agent| { - let Some(agent) = agent else { - return Err(crate::Error::<T>::AgentDoesNotExist.into()); - }; - - if is_in_update_cooldown::<T>(&agent_key)? { - return Err(crate::Error::<T>::AgentUpdateOnCooldown.into()); - } + if is_in_update_cooldown::<T>(&agent_key)? { + return Err(crate::Error::<T>::AgentUpdateOnCooldown.into()); + } + + crate::Agents::<T>::try_mutate(&agent_key, |agent| { + let Some(agent) = agent else { + return Err(crate::Error::<T>::AgentDoesNotExist.into()); + };
225-225: Consider using checked arithmetic for block number calculations.Using
saturating_addfor block number calculations could hide overflow issues. Consider usingchecked_addto explicitly handle potential overflows.- Ok(last_update.saturating_add(cooldown) > current_block) + Ok(last_update.checked_add(cooldown).ok_or(crate::Error::<T>::Overflow)? > current_block)pallets/permission0/api/src/lib.rs (1)
129-142: Clarify why APPLICATION_REVIEW starts at bit position 1 instead of 0.The bitflags start at
0b0000_0010(bit 1) instead of0b0000_0001(bit 0). If this is intentional (e.g., reserving bit 0 for future use), consider adding a comment. Otherwise, consider starting from bit 0.pallets/permission0/src/ext/emission_impl.rs (2)
142-154: Check balance availability before performing expensive validations.For
FixedAmountallocations, the balance check and reservation happen after potentially expensive validations. Consider checkingcan_reserveearly to fail fast if funds are insufficient.Move the balance check before other validations for better performance when funds are insufficient.
363-366: Improve error handling clarity for allocation type validation.The code returns
NotEditableerror but the check forEmissionAllocation::Streamshappens later. Consider validating the allocation type earlier for clearer error messages.pallets/torus0/src/lib.rs (1)
389-396: Consider reordering validation checks for better UX.The namespace path validation happens before checking if the namespace is being delegated. If delegation check is cheaper, consider doing it first to provide more specific error messages sooner.
pallets/governance/src/lib.rs (1)
399-399: Consider returning previous state from toggle operations.The toggle operations directly negate the current state. Consider storing and including the previous state in the event for better auditability.
- let new_state = !crate::AgentsFrozen::<T>::get(); + let previous_state = crate::AgentsFrozen::<T>::get(); + let new_state = !previous_state;Also applies to: 418-418
pallets/governance/src/proposal.rs (1)
437-444: Consider making the proposal processing interval configurableThe 100-block interval for processing proposals is hardcoded. Consider making this a configurable parameter in
GovernanceConfigurationfor better flexibility.Add a new field to
GovernanceConfiguration:pub struct GovernanceConfiguration<T: Config> { // ... existing fields ... pub proposal_processing_interval: BlockNumberFor<T>, }Then update the check:
- if block_number_u64 % 100 != 0 { + let interval: u64 = governance_config.proposal_processing_interval + .try_into() + .ok() + .unwrap_or(100); + if block_number_u64 % interval != 0 {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Cargo.lockis excluded by!**/*.lockflake.lockis excluded by!**/*.lock
📒 Files selected for processing (107)
.devcontainer/devcontainer.json(1 hunks).github/CODEOWNERS(1 hunks).github/workflows/build-docker-node.yml(2 hunks).github/workflows/build-docker-xtask.yml(1 hunks).github/workflows/build-docs.yml(1 hunks).github/workflows/build-runtime.yml(2 hunks).github/workflows/check.yml(2 hunks).gitignore(1 hunks).vscode/extensions.json(1 hunks).vscode/settings.json(1 hunks)CLAUDE.md(1 hunks)CONTRIBUTING.md(1 hunks)Cargo.toml(2 hunks)README.md(1 hunks)docker-compose.yml(1 hunks)docker/xtask.dockerfile(1 hunks)docs/deploy.md(1 hunks)docs/development/git-workflow.md(1 hunks)docs/linear-emission.md(1 hunks)docs/namespace.md(1 hunks)docs/pallet-structure.md(1 hunks)docs/permission0.md(1 hunks)docs/xtask-manual.md(1 hunks)flake.nix(3 hunks)justfile(2 hunks)node/Cargo.toml(2 hunks)node/src/chain_spec.rs(0 hunks)node/src/cli/eth.rs(2 hunks)node/src/command.rs(1 hunks)node/src/command/benchmarking.rs(2 hunks)node/src/rpc.rs(2 hunks)node/src/rpc/eth.rs(3 hunks)node/src/service.rs(3 hunks)pallets/emission0/Cargo.toml(2 hunks)pallets/emission0/api/Cargo.toml(1 hunks)pallets/emission0/api/src/lib.rs(2 hunks)pallets/emission0/src/benchmarking.rs(1 hunks)pallets/emission0/src/benchmarks.rs(0 hunks)pallets/emission0/src/distribute.rs(17 hunks)pallets/emission0/src/distribute/math.rs(6 hunks)pallets/emission0/src/ext.rs(1 hunks)pallets/emission0/src/lib.rs(6 hunks)pallets/emission0/src/migrations.rs(1 hunks)pallets/emission0/src/weight_control.rs(3 hunks)pallets/emission0/src/weights.rs(3 hunks)pallets/emission0/tests/distribution.rs(18 hunks)pallets/emission0/tests/weights.rs(4 hunks)pallets/faucet/Cargo.toml(1 hunks)pallets/faucet/src/ext.rs(1 hunks)pallets/faucet/src/faucet.rs(1 hunks)pallets/faucet/src/lib.rs(1 hunks)pallets/faucet/tests/faucet.rs(1 hunks)pallets/governance/Cargo.toml(1 hunks)pallets/governance/api/Cargo.toml(1 hunks)pallets/governance/api/src/lib.rs(1 hunks)pallets/governance/src/application.rs(10 hunks)pallets/governance/src/benchmarking.rs(1 hunks)pallets/governance/src/benchmarks.rs(0 hunks)pallets/governance/src/config.rs(1 hunks)pallets/governance/src/ext.rs(0 hunks)pallets/governance/src/lib.rs(18 hunks)pallets/governance/src/migrations.rs(1 hunks)pallets/governance/src/proposal.rs(24 hunks)pallets/governance/src/roles.rs(2 hunks)pallets/governance/src/voting.rs(4 hunks)pallets/governance/src/weights.rs(12 hunks)pallets/governance/src/whitelist.rs(2 hunks)pallets/governance/tests/application.rs(14 hunks)pallets/governance/tests/config.rs(1 hunks)pallets/governance/tests/curator.rs(0 hunks)pallets/governance/tests/delegation.rs(1 hunks)pallets/governance/tests/proposal.rs(1 hunks)pallets/governance/tests/roles.rs(1 hunks)pallets/governance/tests/voting.rs(7 hunks)pallets/permission0/Cargo.toml(1 hunks)pallets/permission0/api/Cargo.toml(1 hunks)pallets/permission0/api/src/lib.rs(1 hunks)pallets/permission0/rpc/Cargo.toml(1 hunks)pallets/permission0/rpc/src/lib.rs(1 hunks)pallets/permission0/src/benchmarking.rs(1 hunks)pallets/permission0/src/ext.rs(1 hunks)pallets/permission0/src/ext/curator_impl.rs(1 hunks)pallets/permission0/src/ext/emission_impl.rs(1 hunks)pallets/permission0/src/ext/namespace_impl.rs(1 hunks)pallets/permission0/src/lib.rs(1 hunks)pallets/permission0/src/migrations.rs(1 hunks)pallets/permission0/src/permission.rs(1 hunks)pallets/permission0/src/permission/curator.rs(1 hunks)pallets/permission0/src/permission/emission.rs(1 hunks)pallets/permission0/src/weights.rs(1 hunks)pallets/permission0/tests/curator.rs(1 hunks)pallets/permission0/tests/enforcement.rs(1 hunks)pallets/permission0/tests/fixed.rs(1 hunks)pallets/permission0/tests/lifetime.rs(1 hunks)pallets/permission0/tests/stream.rs(1 hunks)pallets/torus0/Cargo.toml(1 hunks)pallets/torus0/api/Cargo.toml(1 hunks)pallets/torus0/api/src/api.rs(1 hunks)pallets/torus0/api/src/lib.rs(3 hunks)pallets/torus0/rpc/Cargo.toml(1 hunks)pallets/torus0/rpc/src/lib.rs(1 hunks)pallets/torus0/src/agent.rs(9 hunks)pallets/torus0/src/benchmarking.rs(1 hunks)pallets/torus0/src/burn.rs(3 hunks)pallets/torus0/src/fee.rs(1 hunks)pallets/torus0/src/lib.rs(14 hunks)pallets/torus0/src/migrations.rs(1 hunks)
⛔ Files not processed due to max files limit (4)
- pallets/torus0/src/namespace.rs
- pallets/torus0/src/stake.rs
- pallets/torus0/src/weights.rs
- pallets/torus0/tests/agent.rs
💤 Files with no reviewable changes (5)
- node/src/chain_spec.rs
- pallets/governance/src/ext.rs
- pallets/emission0/src/benchmarks.rs
- pallets/governance/tests/curator.rs
- pallets/governance/src/benchmarks.rs
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/build-docs.yml
14-14: label "ubicloud-standard-2" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/build-runtime.yml
19-19: label "ubicloud-standard-16" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/build-docker-node.yml
20-20: label "ubicloud-standard-16" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/check.yml
21-21: label "ubicloud-standard-8" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
.github/workflows/build-docker-xtask.yml
19-19: label "ubicloud-standard-2" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🪛 YAMLlint (1.37.1)
.github/workflows/build-docs.yml
[error] 7-7: trailing spaces
(trailing-spaces)
[error] 20-20: trailing spaces
(trailing-spaces)
🪛 LanguageTool
CLAUDE.md
[uncategorized] ~14-~14: Loose punctuation mark.
Context: ...g guide ## Core Pallets - torus0: Agent registration, staking, burn mecha...
(UNLIKELY_OPENING_PUNCTUATION)
docs/deploy.md
[uncategorized] ~11-~11: The preposition “in” seems more likely in this position than the preposition “on”.
Context: ...cksum. - The .zip file can be found on the Artifacts section of the Action t...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_ON_IN)
[uncategorized] ~13-~13: The preposition “in” seems more likely in this position than the preposition “on”.
Context: ... column. - The WASM blob to be used on the next steps is inside the downloaded...
(AI_EN_LECTOR_REPLACEMENT_PREPOSITION_ON_IN)
[typographical] ~22-~22: It seems that a comma is missing after this introductory phrase.
Context: ...the 4th step. - Sign and Submit. > As of april 9th 2025 the try-runtime cli is broken,...
(AS_OF_COMMA)
docs/development/git-workflow.md
[uncategorized] ~20-~20: Possible missing comma found.
Context: ...re especially important in a blockchain setting where code correctness is not just a ma...
(AI_HYDRA_LEO_MISSING_COMMA)
[grammar] ~26-~26: Possible agreement error. Did you mean “commons”?
Context: ...ge Strategy Trade-offs There are three common merge strategies: ### 1. Squash Merge ...
(THERE_RE_MANY)
[style] ~36-~36: To elevate your writing, try using a synonym here.
Context: ...ity and authorship detail. - Makes it hard to know what happened inside a PR. - ...
(HARD_TO)
docs/pallet-structure.md
[uncategorized] ~134-~134: Possible missing comma found.
Context: ...ins require careful handling of storage changes since data persists across runtime upgr...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~170-~170: Possible missing comma found.
Context: ...data. Agents can be registered by anyone but must meet certain criteria, such as pas...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~222-~222: This phrase is redundant. Consider writing “evolve”.
Context: ...maintainable blockchain system that can evolve over time while maintaining backward compatibilit...
(EVOLVE_OVER_TIME)
docs/linear-emission.md
[uncategorized] ~9-~9: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...ew block (target: 8 seconds), a certain amount of tokens is added to the `PendingEmiss...
(AMOUNTOF_TO_NUMBEROF)
[style] ~11-~11: ‘taken into account’ might be wordy. Consider a shorter alternative.
Context: ...mission recycling percentage. Those are taken into account inside the `get_total_emission_per_bloc...
(EN_WORDINESS_PREMIUM_TAKEN_INTO_ACCOUNT)
[uncategorized] ~15-~15: You might be missing the article “the” here.
Context: ...e, subject to change via proposals, but default value is 100 blocks. We use `pallet-...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[locale-violation] ~17-~17: In American English, ‘afterward’ is the preferred variant. ‘Afterwards’ is more commonly used in British English and other dialects.
Context: ...yAPI to issue tokens and deposit them afterwards. This is very important. Theissue` fu...
(AFTERWARDS_US)
[style] ~17-~17: As an alternative to the over-used intensifier ‘very’, consider replacing this phrase.
Context: ...ns and deposit them afterwards. This is very important. The issue functions returns a `Negat...
(EN_WEAK_ADJECTIVE)
[uncategorized] ~31-~31: Possible missing comma found.
Context: ...ollowing order: 1. iterate through all agents delegating weight control to other vali...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~32-~32: Possible missing comma found.
Context: ...ator; 2. iterate through all registered agents removing from the consensus member list...
(AI_HYDRA_LEO_MISSING_COMMA)
[typographical] ~32-~32: The word “otherwise” is an adverb that can’t be used like a conjunction, and therefore needs to be separated from the sentence.
Context: ...t, an input with the correct weights is created, otherwise, an empty input is used with no weights;...
(THUS_SENTENCE)
[uncategorized] ~35-~35: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...arger than the MinValidatorStake value and it has weights set, the weights are fil...
(COMMA_COMPOUND_SENTENCE)
[uncategorized] ~35-~35: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...t weights set on themselves are excluded and the remaining ones only refer to regist...
(COMMA_COMPOUND_SENTENCE_2)
[uncategorized] ~57-~57: It seems likely that a singular genitive (’s) apostrophe is missing.
Context: ...es for compact storage in the consensus members record. 9. Finally, the emissions are ...
(AI_HYDRA_LEO_APOSTROPHE_S_XS)
CONTRIBUTING.md
[uncategorized] ~44-~44: Loose punctuation mark.
Context: ...nized into 5 main crates: - node/: Contains the actual node implementation...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~46-~46: Loose punctuation mark.
Context: ...rplate from substrate. - pallets/: Contains the runtime logic modules sepa...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~48-~48: Loose punctuation mark.
Context: ...ated by responsibility: - emission0: handles all token emission logic; - `...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~52-~52: Loose punctuation mark.
Context: ...ions with the network. - runtime/: Defines the configuration used by the n...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~56-~56: Possible missing comma found.
Context: ...delines We follow standard Rust coding conventions with some additional requirements speci...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~64-~64: A determiner appears to be missing. Consider inserting it.
Context: ...encies to maintain version consistency. Last thing you want is a massive Cargo file ...
(AI_EN_LECTOR_MISSING_DETERMINER)
docs/namespace.md
[uncategorized] ~68-~68: Possible missing comma found.
Context: ... Agents think carefully about namespace creation since deposits are locked. The base fee...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~121-~121: Possible missing comma found.
Context: ... to agent.memory.twitter to analytics agents while keeping other sources private. A...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~123-~123: Possible missing comma found.
Context: ...cture naturally represents the hardware taxonomy while permissions control access. API ...
(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~135-~135: Use a comma before “and” if it connects two independent clauses (unless they are closely connected and short).
Context: ...future versions as usage patterns emerge and economic requirements become clearer. ...
(COMMA_COMPOUND_SENTENCE_2)
[misspelling] ~149-~149: Use “an” instead of ‘a’ if the following word starts with a vowel sound, e.g. ‘an article’, ‘an hour’.
Context: ...# Security Considerations We will need a anti-spam system to emerge in the near ...
(EN_A_VS_AN)
docs/xtask-manual.md
[uncategorized] ~139-~139: Possible missing comma found.
Context: ...nnect to each other and start producing blocks since they're both configured as valida...
(AI_HYDRA_LEO_MISSING_COMMA)
[style] ~151-~151: Consider shortening or rephrasing this to strengthen your wording.
Context: ...y cover the codebase, particularly when making changes to the pallets. ## Internal Implementatio...
(MAKE_CHANGES)
README.md
[uncategorized] ~69-~69: Loose punctuation mark.
Context: ...operates on two primary ports: - 9944: a WebSocket RPC for interacting with th...
(UNLIKELY_OPENING_PUNCTUATION)
[uncategorized] ~70-~70: Loose punctuation mark.
Context: ...e node behind a reverse proxy. - 30333: Used for node-to-node communication in ...
(UNLIKELY_OPENING_PUNCTUATION)
[style] ~157-~157: ‘in support of’ might be wordy. Consider a shorter alternative.
Context: ...Stake | Tokens locked in support of an agent. Higher stakes affect the weig...
(EN_WORDINESS_PREMIUM_IN_SUPPORT_OF)
docs/permission0.md
[uncategorized] ~5-~5: ‘Amount of’ should usually only be used with uncountable or mass nouns. Consider using “number” if this is not the case.
Context: ...entage of incoming emissions or a fixed amount of tokens. The concept draws inspirati...
(AMOUNTOF_TO_NUMBEROF)
[grammar] ~30-~30: ‘a’ may be redundant when used with the uncountable noun ‘permission’.
Context: ...bles recursive delegation chains, where a permission can be derived from a higher-level perm...
(A_UNCOUNTABLE_NOUN)
[grammar] ~30-~30: ‘a’ may be redundant when used with the uncountable noun ‘permission’.
Context: ... where a permission can be derived from a higher-level permission. This creates hierarchical delegation t...
(A_UNCOUNTABLE_NOUN)
[style] ~93-~93: As a shorter alternative for ‘able to’, consider using “can”.
Context: ... future use. For now, only the SUDO key is able to grant curator permissions. The `cooldo...
(BE_ABLE_TO)
[uncategorized] ~112-~112: Loose punctuation mark.
Context: ...ns flow through the network: - Manual: The grantee must explicitly call `execu...
(UNLIKELY_OPENING_PUNCTUATION)
[grammar] ~130-~130: ‘a’ may be redundant when used with the uncountable noun ‘permission’.
Context: ...rocessing. Revocation terms define how a permission can be revoked before its normal expira...
(A_UNCOUNTABLE_NOUN)
[grammar] ~144-~144: ‘a’ may be redundant when used with the uncountable noun ‘permission’.
Context: ... parties. The grantee can ALWAYS revoke a permission as it is the one being benefitted. ## ...
(A_UNCOUNTABLE_NOUN)
[grammar] ~297-~297: ‘a’ may be redundant when used with the uncountable noun ‘permission’.
Context: ...> DispatchResult ``` Manually executes a permission with distribution control set to Manual...
(A_UNCOUNTABLE_NOUN)
[style] ~297-~297: To form a complete sentence, be sure to include a subject.
Context: ...the targets according to their weights. Can only be called by the grantor or root. ...
(MISSING_IT_THERE)
[grammar] ~309-~309: ‘an’ may be redundant when used with the uncountable noun ‘permission’.
Context: ...` Enables or disables accumulation for an emission permission. Can be called by the grantor, root, or...
(A_UNCOUNTABLE_NOUN)
[style] ~309-~309: To form a complete sentence, be sure to include a subject.
Context: ...ccumulation for an emission permission. Can be called by the grantor, root, or enfo...
(MISSING_IT_THERE)
[grammar] ~320-~320: ‘a’ may be redundant when used with the uncountable noun ‘permission’.
Context: ...onId, ) -> DispatchResult ``` Executes a permission through the enforcement authority mecha...
(A_UNCOUNTABLE_NOUN)
[grammar] ~333-~333: ‘a’ may be redundant when used with the uncountable noun ‘permission’.
Context: ...r updates the enforcement authority for a permission. Can only be called by the grantor or r...
(A_UNCOUNTABLE_NOUN)
[style] ~333-~333: To form a complete sentence, be sure to include a subject.
Context: ...enforcement authority for a permission. Can only be called by the grantor or root. ...
(MISSING_IT_THERE)
[style] ~510-~510: As a shorter alternative for ‘able to’, consider using “can”.
Context: ...isms By using the Currency trait, we are able to use reserves for fixed amount emissions...
(BE_ABLE_TO)
[grammar] ~510-~510: Using ‘plenty’ without ‘of’ is considered to be informal.
Context: ..., we are able to use reserves for fixed amount emissions, and negative imbalances to avoid emitt...
(PLENTY_OF_NOUNS)
🪛 markdownlint-cli2 (0.17.2)
docs/pallet-structure.md
21-21: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: check
- GitHub Check: check
🔇 Additional comments (157)
pallets/emission0/api/Cargo.toml (1)
9-10: Workspace-level linting activated – confirm root configuration is present
workspace = truedelegates lint configuration to the workspace root.
If the rootCargo.toml(or alints.toml) does not define the desired lint set,cargo check -Zlintwill fail.Run once from repo root:
#!/bin/bash # Locate a workspace-level [lints] section rg --max-filesize 10K -n $'\[lints\]' Cargo.tomlYou should see an entry in the root manifest; if nothing is returned, add:
[lints] workspace = true…plus the actual lint rules.
Otherwise, 👍 the change is good..github/CODEOWNERS (1)
1-1: Owner hand-off acknowledged
@supremesource→@functor-flowupdate looks consistent with recent org changes. No action required.node/src/command.rs (1)
24-28: Import re-ordering looks goodPurely stylistic move – no functional impact. Keeps local crate imports grouped together, which improves readability.
.vscode/extensions.json (1)
4-6: New coverage-gutters extension is a nice DX additionNo objections. 👍
(Remember to add the corresponding settings in.vscode/settings.json, which it looks like you already did in this PR.)pallets/torus0/src/fee.rs (1)
4-6: Consolidateduseblock improves clarityThe grouped import keeps
polkadot_sdkitems together – small but helpful for scanability.node/src/command/benchmarking.rs (2)
5-5: Nit: maintain alphabetical orderingIf we keep
std::time-first or alphabetical, the diff noise goes away next time. Current order (Arc, Duration) is fine but consider:-use std::{sync::Arc, time::Duration}; +use std::{sync::Arc, time::Duration};Actually identical – looks like only move relative to other imports. No action required.
18-18: Import path intact after move
FullClientstill resolves; build unaffected.node/src/cli/eth.rs (2)
28-30: Doc comment reflow – goodSplitting long comment into 2 lines matches rust-fmt output; no behaviour change.
54-56: Doc comment reflow – goodSame here.
pallets/governance/Cargo.toml (2)
49-49: LGTM: Clean API dependency addition.The addition of
pallet-permission0-apito workspace dependencies follows the established pattern and is properly placed for runtime integration.
53-53: LGTM: Appropriate test dependency placement.Adding
pallet-permission0to dev-dependencies is the correct approach for testing integration without including the full pallet in the runtime build.docker-compose.yml (2)
4-4: Verify the implications of using thelatesttag.Changing from a fixed commit hash (
f6efa2e) tolatestreduces deployment reproducibility and could introduce instability if the latest image contains breaking changes.Consider using semantic versioning tags (e.g.,
v1.2.3) or reverting to commit-specific tags for production deployments to ensure consistent, reproducible deployments.
12-29: LGTM: Improved command readability.The multi-line YAML format significantly improves readability while preserving all original command arguments and their values.
.vscode/settings.json (2)
5-11: LGTM: Enhanced development configuration.The reformatted environment variables improve readability, and the addition of the "testnet" feature aligns with the broader runtime enhancements in this PR.
12-26: LGTM: Useful development tooling additions.The coverage gutters configuration and project-specific spell checker dictionary will improve the development experience and code quality.
node/Cargo.toml (2)
16-16: LGTM: Consistent feature definition.The testnet feature follows the established pattern and properly delegates to the runtime's testnet feature.
109-110: LGTM: RPC dependencies added for new pallets.The addition of RPC workspace dependencies for permission0 and torus0 pallets is necessary for the RPC integration and follows the established dependency management pattern.
node/src/rpc.rs (2)
25-25: LGTM: Clean RPC module imports.The imports are properly structured and the Arc import organization improvement enhances code clarity.
Also applies to: 30-31
89-91: LGTM: Proper RPC integration with error handling.The new RPC modules are correctly instantiated with cloned client references and properly merged into the main RPC interface. The existing error propagation pattern is maintained with the
?operator.pallets/emission0/api/src/lib.rs (2)
3-3: LGTM: Appropriate import for new functionality.The
DispatchResultimport is correctly added to support the new delegation method.
17-17: LGTM: Well-designed delegation API method.The method signature follows Substrate conventions with reference parameters and
DispatchResultreturn type, which is appropriate for operations that can fail.pallets/torus0/rpc/Cargo.toml (1)
1-15: LGTM: Well-structured RPC crate manifest.The manifest follows standard Substrate RPC crate patterns with appropriate dependencies and workspace configuration.
.devcontainer/devcontainer.json (1)
1-9: LGTM: Solid development container setup.The configuration uses a reliable base image and integrates Nix with direnv for reproducible development environments, which aligns well with blockchain development best practices.
pallets/faucet/src/ext.rs (1)
1-8: LGTM: Standard Substrate type aliases.The type aliases follow established Substrate patterns for extracting associated types from trait bounds, with appropriate visibility and correct implementations.
pallets/torus0/api/Cargo.toml (3)
9-11: LGTM: Consistent workspace linting configuration.Adding workspace-level linting maintains consistency across the codebase.
14-14: LGTM: Expanded std feature dependencies.Including
codec/stdandscale-info/stdin the std feature is appropriate for comprehensive API functionality.
19-26: LGTM: Enhanced dependency configuration.The workspace dependency usage and expanded polkadot-sdk features (sp-api, sp-core, sp-runtime, sp-std) properly support the enhanced API capabilities.
pallets/torus0/Cargo.toml (1)
19-22: Double-check feature gating after dependency shuffle
substrate-fixedwas removed in favour oflibm; ensure all floating-point helpers now come exclusively fromlibm, otherwiseno_stdbuilds will fail at link time.Also, the newly introduced empty
testnetfeature is referenced by the runtime and CI, but the pallet itself has no conditional code behind it. Confirm this is intentional; if the feature should be a pure “pass-through”, add:[features] testnet = [] # ← clarify intention with a commentor wire it to any test-net specific cfg flags.
Also applies to: 28-28, 34-40
pallets/governance/tests/delegation.rs (2)
5-16: LGTM! Test logic is correct and well-structured.The test properly validates that enabling vote delegation removes the module from the
NotDelegatingVotingPowerstorage set, which is the expected behavior.
18-33: LGTM! Complete delegation lifecycle test.This test properly validates the full delegation lifecycle - enabling delegation first, then disabling it, and confirming the module is back in the
NotDelegatingVotingPowerset. The test flow is logical and comprehensive.pallets/emission0/Cargo.toml (3)
19-21: LGTM! Consistent feature dependency additions.The addition of
pallet-governance-api/stdandpallet-permission0-api/stdto the std feature list is consistent with the new dependencies being added to support the permission system integration.
26-27: LGTM! Runtime benchmarks feature updated consistently.The runtime-benchmarks feature correctly includes the new API dependencies, ensuring benchmarking capabilities are available for the integrated permission and governance systems.
42-42: LGTM! New permission API dependency added.The addition of
pallet-permission0-apias a workspace dependency is consistent with the broader integration of the permission system into the emission pallet.pallets/emission0/src/ext.rs (3)
1-1: LGTM! Import expansion for consistency.Adding
frame_systemto the import statement supports the consistent use offrame_system::Configthroughout the type aliases, improving code readability and maintenance.
3-6: LGTM! Improved consistency and visibility.The changes improve the codebase by:
- Making type aliases
pubinstead ofpub(super)for broader accessibility- Using consistent
frame_system::Configreferences- Maintaining the same underlying functionality while improving usability
8-10: LGTM! New type alias supports emission logic.The addition of
NegativeImbalanceOf<T>type alias provides a clean abstraction for the negative imbalance type, which is likely needed for the enhanced emission distribution logic that integrates with the permission system.CLAUDE.md (1)
1-35: LGTM! Excellent AI guidance documentation.This documentation provides comprehensive and valuable guidance for AI-assisted development:
- Clear project structure overview with relevant file references
- Accurate descriptions of core pallets and their responsibilities
- Practical development commands using the project's tooling
- Critical safety rules that are essential for blockchain runtime development
The content is well-organized, accurate, and will significantly help with AI-assisted code understanding and development in this Substrate blockchain project.
.github/workflows/build-docker-node.yml (4)
1-1: LGTM! More descriptive workflow name.The updated name "Build Torus node Docker image" is more specific and clearly identifies what this workflow builds, improving maintainability.
5-9: LGTM! Clean branch trigger formatting.The multi-line format for branch triggers improves readability, and adding the
devbranch makes sense for comprehensive CI/CD coverage.
22-22: LGTM! Checkout action updated to latest version.Updating from
actions/checkout@v3toactions/checkout@v4follows best practices for using the latest stable action versions.
20-20: Verify the custom runner label availability.The workflow uses
ubicloud-standard-16as the runner, but static analysis indicates this is not a standard GitHub-hosted runner label.#!/bin/bash # Check if this is a self-hosted runner configuration echo "Checking for runner configuration files..." fd -t f "actionlint.ya?ml" . fd -t f "\.github.*runner" .pallets/governance/tests/config.rs (2)
7-26: Well-structured test for emission parameter configuration.The test correctly verifies that the
set_emission_paramsextrinsic updates both the governance pallet'sTreasuryEmissionFeeand the emission pallet'sEmissionRecyclingPercentagewhen called by root origin. Good test coverage for the cross-pallet state updates.
28-40: Proper access control testing.The test correctly verifies that non-root origins are rejected with
BadOriginerror, ensuring proper access control enforcement.pallets/governance/src/config.rs (2)
2-5: Improved import organization and type precision.The addition of
BlockNumberForimport and updated import grouping improves code organization.
14-19: Enhanced type safety with BlockNumberFor.Replacing the generic type alias with
BlockNumberFor<T>provides better type precision for block-related durations. This aligns with Substrate's type system best practices and improves consistency across the runtime.node/src/rpc/eth.rs (2)
3-7: Clean import organization for Frontier components.The grouping of Frontier-related
pub usedeclarations improves code organization and readability.
95-99: Appropriate scoping of fc_rpc imports.Moving the
fc_rpcimports inside thecreate_ethfunction scope is good practice, as these types are only used within this function.pallets/torus0/api/src/api.rs (1)
6-12: Well-designed runtime API for namespace operations.The
Torus0RuntimeApiis properly structured with:
- Appropriate codec bounds for cross-boundary communication
- Clear method documentation
- Proper error handling with
DispatchError- Meaningful return type
(Balance, Balance)for fee and depositnode/src/service.rs (3)
156-163: Excellent type alias for complex future type.The
AuraDatatype alias significantly improves readability by abstracting the complexPin<Box<dyn Future<...>>>type. This makes the code much more maintainable.
165-168: Cleaner function signature with type alias.Using the
AuraDatatype alias in the function signature makes it much more readable and maintainable compared to the raw complex future type.
498-502: Improved comment formatting.The reformatted comment provides clearer explanation of the pubsub notification sink architecture and synchronization strategy.
pallets/faucet/Cargo.toml (1)
1-37: LGTM! Well-structured pallet manifest.The Cargo.toml follows standard Substrate pallet conventions with appropriate workspace configuration, feature flags, and dependencies. The
testnetfeature flag correctly aligns with the pallet's intended testnet-only usage.pallets/permission0/api/Cargo.toml (1)
1-32: LGTM! Properly configured API crate manifest.The Cargo.toml follows best practices for a Substrate API crate with appropriate no_std compatibility, feature flags, and workspace configuration. Dependencies are correctly configured for an API that defines traits and types.
docs/deploy.md (1)
1-23: Excellent deployment guide with strong security practices.The deployment guide is comprehensive and includes important security measures like checksum verification and proper testing procedures. The step-by-step approach with clear versioning guidance will help ensure safe deployments.
pallets/permission0/tests/fixed.rs (1)
1-219: Excellent comprehensive test suite for fixed emission permissions.The test suite provides thorough coverage of fixed emission permission functionality including:
- Error handling for insufficient balance scenarios
- Successful permission creation and storage verification
- Balance reservation mechanics
- Manual and automatic execution flows
- Single-execution constraints
- Multi-agent distribution logic
The tests follow good practices with clear setup, execution, and verification phases, and properly test both success and failure paths.
pallets/torus0/src/burn.rs (3)
2-10: LGTM! Clean migration to standardized fixed-point types.The migration from
substrate-fixedtosp_runtime::FixedU128aligns with the workspace-wide standardization mentioned in the summary. The import consolidation improves maintainability.
41-45: Excellent documentation addition.The doc comment clearly explains the function's purpose and behavior, improving code maintainability.
71-93: Verify numeric precision in fixed-point conversion.The arithmetic logic looks correct, but ensure that the conversion between the old
I110F18(110 integer bits, 18 fractional bits) andFixedU128(64 integer bits, 64 fractional bits) maintains the expected precision for burn calculations.#!/bin/bash # Description: Search for any remaining references to substrate-fixed types or potential precision issues # Expected: No remaining substrate-fixed imports, consistent FixedU128 usage echo "Checking for remaining substrate-fixed references:" rg -i "substrate.?fixed|I110F18|I96F32" --type rust echo "Checking FixedU128 usage patterns:" rg "FixedU128::(from_inner|into_inner|from_u32)" --type rust -A 2 -B 2pallets/emission0/src/weights.rs (3)
5-5: LGTM! Updated benchmarking data reflects recent changes.The updated benchmark date (2025-06-18) and increased storage reads align with the permission system integration mentioned in the summary.
80-87: Storage access pattern changes look appropriate.The addition of
Governance::WhitelistandGovernance::Allocatorsreads indelegate_weight_controlaligns with the permission system integration, ensuring proper authorization before delegation.
49-70: Verify the significant increase in database reads.The
set_weightsfunction now performs 10 database reads (up from 8), and execution time increased from ~27ms to ~33ms. This suggests additional permission and governance checks were added.#!/bin/bash # Description: Verify the storage accesses in set_weights implementation match the benchmark # Expected: Confirm the actual storage reads in the implementation echo "Searching for set_weights implementation:" ast-grep --pattern 'fn set_weights($$$) { $$$ }' echo "Checking storage access patterns in emission0 pallet:" rg "T::DbWeight::get\(\)\.reads\(" pallets/emission0/src/ -A 1 -B 1pallets/permission0/Cargo.toml (3)
1-11: LGTM! Standard pallet manifest structure.The package metadata follows workspace conventions with proper linting configuration and MIT-0 license, which is appropriate for blockchain runtime pallets.
12-27: Well-configured feature flags.The feature structure properly propagates
std,runtime-benchmarks, andtry-runtimefeatures to dependencies, ensuring consistent build configurations across the workspace.
29-40: Verify API dependencies align with pallet integration.The dependencies include APIs for
torus0,emission0, andpermission0-api, which aligns with the permission system's cross-pallet integration mentioned in the summary.#!/bin/bash # Description: Verify that these API crates exist and are properly defined in the workspace # Expected: All API crates should be present in workspace Cargo.toml echo "Checking workspace members for permission-related crates:" rg "permission0|torus0.*api|emission0.*api" Cargo.toml -A 2 -B 2 echo "Verifying API crate directories exist:" fd "api" pallets/ --type d.github/workflows/build-docker-xtask.yml (4)
3-12: Well-configured trigger conditions.The path-based filtering ensures the workflow only runs when relevant files change, optimizing CI/CD resource usage.
19-19: Verify custom runner availability.The workflow uses a custom runner label
ubicloud-standard-2. Ensure this runner is properly configured and available in your GitHub organization.Based on the static analysis hint, this appears to be a custom self-hosted runner. Please verify:
- The runner is properly registered and available
- Consider adding this to your actionlint.yaml config if it's a permanent self-hosted runner
- Ensure the runner has necessary Docker and build tools installed
23-30: Docker metadata configuration looks good.The tagging strategy properly handles SHA-based tags for all pushes and
latesttag only for main branch, following best practices for container versioning.
42-50: Excellent caching strategy.The GitHub Actions cache configuration (
type=gha) will significantly improve build times for subsequent runs by caching Docker layers.pallets/governance/api/src/lib.rs (3)
18-20: Well-designed permission check methods.The
can_create_namespace()andcan_register_agent()methods provide clear, boolean permission checks that align with the new permission system integration.
22-24: Proper benchmarking-only method design.The
force_set_whitelisted()method is appropriately feature-gated and marked as hidden documentation, following best practices for benchmarking utilities.
14-24: ```shell
#!/bin/bash
echo "Searching for GovernanceApi impl block:"
rg -n "impl.*GovernanceApi" pallets/governance/src/lib.rs -A3 -B3echo "Verifying set_allocator implementation:"
rg -n "fn set_allocator" pallets/governance/src/lib.rs -A3 -B1</details> <details> <summary>pallets/governance/tests/roles.rs (1)</summary> `39-182`: **Excellent test coverage for governance roles.** The test suite comprehensively covers: - Allocator addition/removal - Permission-based whitelist management - Application-related whitelist restrictions - Agent penalization with proper error handling The tests properly verify both success and failure scenarios with appropriate error assertions. </details> <details> <summary>.github/workflows/check.yml (2)</summary> `60-136`: **Well-structured coverage reporting workflow.** The coverage reporting implementation is comprehensive, including: - Conditional execution based on PR detection - Coverage generation in multiple formats (lcov, HTML) - S3 upload with proper access control - Automated PR commenting The workflow properly handles error cases with `continue-on-error` and includes appropriate environment variable usage. --- `21-21`: **Verify the custom runner availability.** The runner label `ubicloud-standard-8` is not a standard GitHub-hosted runner. Ensure this custom runner is properly configured and available in your environment. ```shell #!/bin/bash # Check if the custom runner is properly configured gh api /repos/$GITHUB_REPOSITORY/actions/runners --jq '.runners[] | select(.labels[] | contains("ubicloud-standard-8"))'pallets/permission0/tests/lifetime.rs (1)
5-240: Comprehensive permission lifetime test coverage.The test suite excellently covers all critical permission lifecycle scenarios:
- Permission expiration validation
- Irrevocable permission enforcement
- Grantor-based revocation
- Time-based revocation restrictions
- Arbiter-based revocation with proper validation
The tests use consistent setup patterns and comprehensive error checking, ensuring robust validation of the permission system's security model.
pallets/governance/src/voting.rs (4)
5-5: Clear and concise documentation.The documentation comment accurately describes the function's purpose without being overly verbose.
56-56: Accurate documentation for vote removal.The documentation clearly explains the function's purpose.
82-82: Good delegation documentation.The comment accurately describes the delegation enabling functionality.
91-91: Consistent delegation documentation.The comment properly describes the delegation disabling functionality.
pallets/emission0/src/weight_control.rs (4)
27-29: Improved access control validation.The combined check for agent registration and whitelist status provides better security by ensuring both conditions are met.
35-38: Simplified stake validation logic.The change from per-weight minimum stake checks to a fixed minimum validator stake simplifies the validation logic while maintaining security requirements.
68-71: More explicit function signature.The change from origin-based to explicit account parameters makes the function signature clearer and more testable.
103-106: Improved error handling for allocator check.The explicit allocator check with specific error return provides better error semantics than the previous implementation.
pallets/permission0/src/migrations.rs (1)
1-17: Verify that zero-weight migration is intentional.Both migration modules return
Weight::zero(), indicating no computational work or state changes during runtime upgrade. If this is a placeholder implementation, consider adding documentation explaining the intended future migration logic.Let me verify if there are any related storage items or migration logic that should be implemented:
#!/bin/bash # Description: Check for storage items in the permission0 pallet that might need migration ast-grep --pattern $'#[pallet::storage] $$$' # Also check for any TODO or FIXME comments related to migrations rg -i "todo|fixme" --type rust -A 3 -B 1pallets/permission0/rpc/src/lib.rs (1)
18-22: LGTM! Well-structured RPC trait definition.The trait definition follows jsonrpsee conventions properly with appropriate async signature and descriptive method naming.
justfile (3)
1-1: Good addition of formatting to the default workflow.Adding
fmtbeforecheckandtestensures code is properly formatted before validation, improving code consistency.
5-9: LGTM! Clear separation of mainnet and testnet builds.The separate build targets properly handle feature flags for different deployment environments.
14-14: Good addition of--testsflag to clippy.Including test code in linting helps maintain code quality across the entire codebase.
pallets/torus0/rpc/src/lib.rs (1)
19-27: LGTM! Well-defined RPC trait with clear parameter types.The trait definition is clear and follows Substrate RPC conventions with appropriate async signature and descriptive naming.
pallets/permission0/tests/curator.rs (3)
11-14: Good use of helper function for common operations.The
ensure_curatorhelper function reduces code duplication and makes tests more readable.
16-65: Comprehensive test coverage for permission granting.The test covers important scenarios including permission validation, duplicate prevention, and ROOT bit filtering. The bit manipulation testing at lines 54-55 and 63 is particularly valuable for security.
102-131: Excellent cooldown mechanism testing.The test properly validates the cooldown functionality by testing before execution, after execution during cooldown, and after cooldown expiry. This is crucial for permission security.
docs/development/git-workflow.md (1)
1-125: Well-structured git workflow documentation!This documentation provides excellent guidance for maintaining traceability in a blockchain codebase. The comparison of merge strategies and the rationale for preferring merge commits is particularly valuable.
pallets/torus0/src/benchmarking.rs (1)
1-134: Comprehensive benchmarking coverage!The benchmarking module properly covers all major extrinsics with appropriate setup for each benchmark.
flake.nix (2)
1-98: LLVM toolchain configuration looks good!The explicit LLVM 17 configuration and PATH prioritization ensures consistent toolchain usage across the project.
5-5: Verify nixpkgs 25.05 availabilityThe update from nixos-24.11 to nixos-25.05 assumes this version exists. Please verify this is a valid nixpkgs channel.
Does NixOS have a 25.05 release channel available?pallets/emission0/src/migrations.rs (1)
7-45: Migration logic handles delegation correctlyThe migration properly delegates weight control from non-allocator agents to the first allocator, with appropriate error handling.
pallets/emission0/src/benchmarking.rs (2)
1-91: Well-structured weight management benchmarks!The benchmarking module properly tests the weight control extrinsics with comprehensive setup using cross-pallet APIs.
17-45: Good benchmark setup for weight settingThe benchmark correctly sets up the allocator with sufficient stake before testing weight setting, which reflects real-world usage.
pallets/governance/src/roles.rs (5)
1-1: LGTM! Permission system integration properly imported.The import correctly brings in the necessary types and traits from the permission0 pallet API for curator permission management.
11-20: LGTM! Proper existence check and error handling.The function correctly prevents duplicate allocator registration with appropriate error handling.
22-31: LGTM! Proper existence validation before removal.The function correctly validates allocator existence before removal, preventing errors on non-existent entries.
33-66: LGTM! Comprehensive permission-based penalty enforcement.The function properly integrates with the permission system, validates penalty percentages, and atomically updates agent data with appropriate event emission.
68-75: LGTM! Simple and effective allocator validation.The function provides clear error handling for allocator verification.
pallets/emission0/tests/weights.rs (4)
16-23: LGTM! Direct function call approach improves test clarity.The change to pass account IDs directly instead of using
get_originmakes the delegation test logic clearer and more direct.
41-45: LGTM! Proper allocator enforcement before delegation.The test correctly verifies that delegation requires the delegatee to be an allocator, which aligns with the security model.
89-89: LGTM! Using pallet API constant improves maintainability.The change from hardcoded values to
MinValidatorStake::<Test>::get()makes the test more maintainable and consistent with pallet configuration.
53-57: Verify the WeightControlNotEnabled error is intentional.The test expects
WeightControlNotEnablederror, suggesting weight control regain is currently disabled. Ensure this aligns with the current pallet state.#!/bin/bash # Description: Verify weight control functionality status in emission pallet. # Expected: Find references to WeightControlNotEnabled error and regain functionality. rg -A 3 -B 3 "WeightControlNotEnabled"Cargo.toml (3)
7-7: LGTM! RPC workspace integration properly added.The addition of
"pallets/*/rpc"to workspace members correctly includes RPC crates in the workspace structure.
35-41: LGTM! New pallet dependencies properly structured.The addition of faucet and permission0 pallets with their API and RPC crates follows the established workspace dependency pattern.
51-57: LGTM! Utility dependencies are well-chosen.The addition of
async-trait,bitflags,rand, andlibmprovides essential utilities for async programming, bitfield operations, randomness, and mathematical functions.CONTRIBUTING.md (1)
1-156: Excellent comprehensive contribution guide!The document provides thorough coverage of development workflow, coding standards, testing requirements, and PR processes. The content is technically accurate and aligns well with the codebase structure and requirements.
pallets/governance/tests/application.rs (5)
7-7: LGTM! Proper permission system import.The import correctly brings in
CuratorPermissionsfor the new permission-based curator system.
19-19: LGTM! Migration to permission-based curator system.The change from direct storage manipulation to
grant_curator_permissionproperly integrates with the new permission0 pallet architecture.
126-126: LGTM! Improved variable naming clarity.The rename from
adding_keytoremoving_keycorrectly reflects the variable's purpose in the removal test.
264-264: LGTM! Proper API usage for treasury address.The change from
Test::dao_treasury_address()toGovernance::dao_treasury_address()correctly uses the governance pallet's API.
273-277: LGTM! Centralized configuration management.The centralized approach to setting and retrieving expiration blocks improves test maintainability and consistency.
pallets/governance/src/weights.rs (1)
1-677: Weights correctly updated for governance pallet refactoring.The autogenerated weights properly reflect the governance pallet's transition to permission-based curator management:
- Removed weights for deprecated
add_curatorandremove_curatorfunctions- Added weights for new
toggle_agent_freezingandtoggle_namespace_freezingfunctions- Updated storage proofs to include
Permission0pallet interactions- Increased weight values appropriately account for additional permission checks
pallets/permission0/src/permission/curator.rs (1)
1-81: Well-structured curator permissions implementation.The bitflags-based permission system is cleanly implemented with:
- Proper bit patterns for each permission flag
- Convenient helper methods for common permission checks
- Appropriate derives for runtime compatibility
- Intentionally empty cleanup method (no special cleanup needed for curator permissions)
pallets/permission0/src/weights.rs (1)
1-288: Comprehensive weights for permission0 pallet operations.The autogenerated weights properly cover all permission management extrinsics with accurate storage proof annotations and reasonable execution times based on benchmarking.
pallets/permission0/src/benchmarking.rs (1)
1-314: Thorough benchmarking coverage for permission extrinsics.The benchmarks properly test all permission lifecycle operations with:
- Realistic test scenarios including agent registration and funding
- Proper permission setup before testing dependent operations
- Coverage of various permission types (emission, curator) and configurations
- Clean use of the
bounded_btree_map!macro for test datapallets/governance/src/benchmarking.rs (1)
45-281: Well-structured governance benchmarks with proper test setup.The benchmarks comprehensively cover all governance extrinsics including:
- Proper account funding and agent registration where needed
- Realistic test data generation respecting constraints
- Coverage of new freezing toggle operations
- Appropriate use of storage setup for voting tests
pallets/faucet/src/lib.rs (1)
203-203: Verify the faucet::execute implementationThe extrinsic delegates to
faucet::execute, but the implementation is not shown in the provided context. Ensure this function properly validates the proof-of-work and handles edge cases.#!/bin/bash # Description: Verify the faucet::execute implementation exists and review its logic # Search for the execute function in the faucet module ast-grep --pattern 'pub fn execute<$_>($$$) { $$$ }' # Also check the faucet module file fd -e rs "faucet.rs" --exec cat {}pallets/permission0/src/ext/namespace_impl.rs (1)
23-24: Clarify namespace delegation logicThe current logic checks if
p == path || path.is_parent_of(p). Should this also check ifp.is_parent_of(path)to handle cases where the grantor has permission on a parent namespace? The intended delegation semantics should be clarified.What are the inheritance rules for namespace permissions in hierarchical systems? Can a parent namespace permission grant access to child namespaces?README.md (1)
1-164: Excellent README transformation!The comprehensive documentation provides clear instructions for running nodes, building from source, network connectivity, and includes a valuable terminology glossary. The structure with table of contents and organized sections greatly improves the developer experience.
pallets/permission0/src/ext.rs (1)
1-221: Well-implemented permission API with robust validation!The external API implementation demonstrates excellent practices:
- Comprehensive parameter validation in translation functions
- Proper enforcement of future block requirements
- Multi-controller voting mechanism with vote tracking
- Clear separation of concerns between API and internal implementations
pallets/torus0/src/migrations.rs (2)
7-64: v4 migration correctly adds the new agent field.The migration properly preserves existing agent data while initializing
last_update_blocktoregistration_block, which is a sensible default.
158-163: Document the reason for using deprecated function.While the
#[allow(deprecated)]attribute suppresses the warning, it would be helpful to add a comment explaining whycreate_namespace0is used here instead of the current namespace creation method.Is there a specific reason this migration needs to use the deprecated
create_namespace0function? Consider adding a comment to document this decision for future maintainers.pallets/emission0/tests/distribution.rs (1)
1-656: Excellent test coverage with proper fixed-point arithmetic migration!The test updates demonstrate thorough coverage:
- Successful migration from
I96F32toFixedU128for consistent fixed-point calculations- New edge case tests for weight delegation scenarios
- Proper verification of
last_dividendsandlast_incentivestracking- Well-structured helper function
set_emissions_paramsfor test setupdocs/xtask-manual.md (1)
1-192: Outstanding documentation for the xtask development tools!This manual provides comprehensive guidance with clear examples, practical workflows, and helpful troubleshooting tips. The structure progresses logically from basic usage to advanced scenarios, making it accessible for developers at all levels.
pallets/governance/src/application.rs (5)
1-29: LGTM! Good type safety improvements.The migration to
BlockNumberFor<T>and consolidatedpolkadot_sdkimports improves type safety and aligns with current Substrate best practices.
38-47: Good formatting and documentation improvement.The multi-line struct style with documentation makes the code more readable and self-documenting.
57-126: Excellent refactoring with improved safety.The use of
saturating_addprevents potential overflow issues, and the comprehensive documentation clearly explains the fee mechanism and application lifecycle.
128-170: Good documentation additions.The doc comment and inline comment explaining the fee refund logic improve code clarity.
192-213: Clean refactoring with proper type usage.The updated parameter type and clear documentation improve the function's interface and maintainability.
pallets/faucet/src/faucet.rs (3)
59-61: Question the manual provider management pattern.The manual check and increment of providers is unusual in Substrate. The frame_system typically handles provider management automatically when an account receives tokens. This pattern might lead to inconsistencies.
Consider removing this manual management and let the
deposit_creatingcall handle it automatically.
140-167: Well-implemented PoW seal generation.The multi-step hashing process provides good entropy mixing, and the explicit use of little-endian encoding for the nonce ensures consistent behavior across platforms.
192-206: Correct PoW difficulty verification.The overflow-based difficulty check is a standard and efficient approach for proof-of-work systems.
pallets/permission0/src/ext/curator_impl.rs (2)
99-158: Well-designed permission granting logic.The implementation includes important security measures:
- Preventing ROOT permission grants (line 110)
- Checking for duplicate permissions (lines 121-129)
- Clear provenance tracking with PalletId as grantor
160-169: Correct permission execution tracking.The use of saturating arithmetic for the execution counter prevents overflow issues.
pallets/faucet/tests/faucet.rs (1)
1-321: Well-structured test configuration.The mock runtime setup is comprehensive, including all necessary pallets and proper parameter configuration.
pallets/emission0/src/distribute.rs (1)
116-118: ```shell
#!/bin/bashRetrieve the definition and documentation (if any) of the is_whitelisted function
rg -n "pub fn is_whitelisted" -A10 pallets/governance/src/whitelist.rs
</details> <details> <summary>pallets/torus0/api/src/lib.rs (2)</summary> `219-296`: **Comprehensive test coverage** The test suite thoroughly covers namespace validation rules, edge cases, and utility methods. Good use of boundary testing for segment lengths. --- `22-22`: ```shell #!/bin/bash # Search for all implementations of the Torus0Api trait in Rust source files rg --with-filename --line-number --context 3 'impl[[:space:]]+Torus0Api<' --glob '*.rs'pallets/permission0/src/permission.rs (3)
32-54: Permission ID generation looks secureThe permission ID generation using Blake2 hash with multiple inputs including block number and extrinsic index provides good uniqueness guarantees. The duplicate check adds an extra safety layer.
215-218: Verify namespace permission cleanup requirementsThe code indicates no cleanup is needed for namespace permissions, but this should be verified to ensure no orphaned data remains.
#!/bin/bash # Description: Check if namespace permissions have any associated storage that needs cleanup # Search for namespace-related storage in the permission pallet rg -A 10 'NamespaceScope|Namespace.*Permission' # Look for namespace delegation storage or indices ast-grep --pattern 'type $_ = Storage$_<$$$NamespaceScope$$$>'
100-113: ```shell
#!/bin/bashLocate the definition of MaxRevokersPerPermission in the pallet
rg -n 'MaxRevokersPerPermission' .
</details> <details> <summary>pallets/governance/tests/voting.rs (1)</summary> `1-381`: **Well-structured test refactoring** The test suite has been successfully refactored to focus on vote management with clear test cases for: - Vote addition and removal - Proposal state validation - Treasury balance verification on proposal closure - Error handling for edge cases Good use of pattern matching for flexible proposal status verification. </details> <details> <summary>pallets/emission0/src/lib.rs (2)</summary> `175-181`: **Improved security with explicit signed origin check** Good change to require a signed origin before delegating weight control. The explicit account parameters also make the API clearer. --- `27-27`: **Storage version bump requires migration** The storage version has been incremented from 1 to 2, indicating breaking storage changes. Ensure a proper migration is implemented. ```shell #!/bin/bash # Description: Verify migration implementation for storage version 2 # Check for migration module cat pallets/emission0/src/migrations.rs # Look for migration tests rg -A 10 'mod.*migration.*test' pallets/emission0/pallets/emission0/src/distribute/math.rs (2)
11-29: Safe numeric operations with FixedU128Excellent migration to
FixedU128with consistent use of saturating arithmetic operations and safe division withconst_checked_div. This prevents overflow/underflow issues critical in blockchain contexts.
136-155: Robust edge case handling in vec_max_upscale_to_u16Good defensive programming:
- Handles empty vectors correctly
- Filters out zero max values
- Uses safe division with fallback
- Proper bounds checking in the conversion
The implementation is safe and handles all edge cases appropriately.
pallets/torus0/src/lib.rs (2)
627-634: Ensure benchmarking-only functions are properly feature-gated.The
force_set_stakefunction bypasses validation and should only be available in benchmarking builds. Verify that the#[cfg(feature = "runtime-benchmarks")]attribute properly prevents this from being included in production builds.#!/bin/bash # Verify that force_set_stake is only used in benchmarking contexts rg -A 5 -B 5 "force_set_stake" --type rust
37-37: I’ll inspect the contents ofmigrations.rsin thetorus0pallet to verify which versions it covers.#!/bin/bash echo "Line count of migrations.rs:" wc -l pallets/torus0/src/migrations.rs echo "----- migrations.rs content -----" cat pallets/torus0/src/migrations.rspallets/governance/src/lib.rs (1)
44-44: ```shell
#!/bin/bashDisplay the full v5 migration module for review
sed -n '210,500p' pallets/governance/src/migrations.rs
Check for any Curators references in the entire file
rg -n "Curators" -C 5 pallets/governance/src/migrations.rs
Check specifically for old_storage usage of Curators in v5
rg -n "old_storage::Curators" -C 10 pallets/governance/src/migrations.rs
</details> <details> <summary>pallets/governance/src/proposal.rs (1)</summary> `710-730`: **Well-implemented quadratic voting reward distribution** The quadratic voting implementation using integer square root is mathematically sound and avoids floating-point precision issues. The use of `FixedU128` for percentage calculations ensures deterministic behavior across all nodes. </details> <details> <summary>docs/permission0.md (1)</summary> `1-517`: **Excellent comprehensive documentation for Permission0 pallet** This documentation provides thorough coverage of the Permission0 pallet's functionality, including: - Clear architectural overview with permission contracts and scopes - Detailed explanation of emission and curator permissions - Well-structured mermaid diagrams illustrating the flow - Comprehensive coverage of all extrinsics and their parameters - Good explanation of the enforcement authority system - Practical examples and use cases The documentation will be valuable for developers integrating with this pallet. </details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
| runs-on: ubicloud-standard-16 | ||
|
|
There was a problem hiding this comment.
runs-on label may break the workflow
ubicloud-standard-16 is not recognised by GitHub-hosted runners and is flagged by actionlint.
If this is a self-hosted runner, prepend the generic label so the job is schedulable:
runs-on: [self-hosted, linux, ubicloud-standard-16]Otherwise the workflow will queue indefinitely/fail.
🧰 Tools
🪛 actionlint (1.7.7)
19-19: label "ubicloud-standard-16" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🤖 Prompt for AI Agents
In .github/workflows/build-runtime.yml at lines 19-20, the runs-on label
"ubicloud-standard-16" is not recognized by GitHub-hosted runners and causes the
workflow to fail or queue indefinitely. To fix this, update the runs-on value to
include the generic self-hosted labels by changing it to runs-on: [self-hosted,
linux, ubicloud-standard-16] so the job can be properly scheduled on the
intended runner.
| runs-on: ubicloud-standard-2 | ||
| steps: |
There was a problem hiding this comment.
Unknown runner label jeopardises docs build
ubicloud-standard-2 is not a standard GitHub label. Use the self-hosted pattern or switch to a GitHub-hosted runner to avoid stuck jobs:
runs-on: [self-hosted, linux, ubicloud-standard-2]If this runner is provided by an external service, add it to actionlint.yaml so static analysis passes.
🧰 Tools
🪛 actionlint (1.7.7)
14-14: label "ubicloud-standard-2" is unknown. available labels are "windows-latest", "windows-latest-8-cores", "windows-2025", "windows-2022", "windows-2019", "ubuntu-latest", "ubuntu-latest-4-cores", "ubuntu-latest-8-cores", "ubuntu-latest-16-cores", "ubuntu-24.04", "ubuntu-24.04-arm", "ubuntu-22.04", "ubuntu-22.04-arm", "ubuntu-20.04", "macos-latest", "macos-latest-xl", "macos-latest-xlarge", "macos-latest-large", "macos-15-xlarge", "macos-15-large", "macos-15", "macos-14-xl", "macos-14-xlarge", "macos-14-large", "macos-14", "macos-13-xl", "macos-13-xlarge", "macos-13-large", "macos-13", "self-hosted", "x64", "arm", "arm64", "linux", "macos", "windows". if it is a custom label for self-hosted runner, set list of labels in actionlint.yaml config file
(runner-label)
🤖 Prompt for AI Agents
In .github/workflows/build-docs.yml at lines 14-15, the runner label
'ubicloud-standard-2' is not recognized by GitHub Actions and may cause the docs
build to get stuck. Update the 'runs-on' field to use the self-hosted runner
pattern by changing it to 'runs-on: [self-hosted, linux, ubicloud-standard-2]'.
If this runner is external, also add it to the actionlint.yaml configuration to
ensure static analysis passes.
| RUN apt-get update && apt install -y --no-install-recommends \ | ||
| ca-certificates \ | ||
| curl \ | ||
| build-essential \ | ||
| protobuf-compiler \ | ||
| libclang-dev \ | ||
| git \ | ||
| pkg-config \ | ||
| libssl-dev |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Remove unnecessary build dependencies from runtime stage.
The runtime stage should only contain minimal dependencies needed to execute the xtask binary. Installing build tools like build-essential, protobuf-compiler, libclang-dev, and pkg-config defeats the purpose of a multi-stage build and significantly increases the final image size.
RUN apt-get update && apt install -y --no-install-recommends \
ca-certificates \
- curl \
- build-essential \
- protobuf-compiler \
- libclang-dev \
- git \
- pkg-config \
- libssl-dev
+ && rm -rf /var/lib/apt/lists/*Only keep ca-certificates unless the xtask binary specifically requires other runtime libraries. Add cleanup of apt cache to reduce image size.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| RUN apt-get update && apt install -y --no-install-recommends \ | |
| ca-certificates \ | |
| curl \ | |
| build-essential \ | |
| protobuf-compiler \ | |
| libclang-dev \ | |
| git \ | |
| pkg-config \ | |
| libssl-dev | |
| RUN apt-get update && apt install -y --no-install-recommends \ | |
| ca-certificates \ | |
| && rm -rf /var/lib/apt/lists/* |
🤖 Prompt for AI Agents
In docker/xtask.dockerfile around lines 23 to 31, remove build dependencies such
as build-essential, protobuf-compiler, libclang-dev, and pkg-config from the
runtime stage to keep the image minimal. Only keep ca-certificates and any other
libraries strictly required by the xtask binary at runtime. Additionally, add
commands to clean up the apt cache after installation to reduce the final image
size.
| polkadot_sdk::sp_tracing::error!("no allocators available"); | ||
| } | ||
|
|
||
| Weight::zero() |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider calculating actual weight for the migration
The migration returns Weight::zero() but performs potentially many storage reads and writes. Consider calculating the actual weight based on the number of agents processed.
fn on_runtime_upgrade() -> Weight {
+ let mut weight = Weight::zero();
+ // Read allocators
+ weight = weight.saturating_add(T::DbWeight::get().reads(1));
+
let allocators: BTreeSet<_> = <T::Governance>::get_allocators().collect();
if let Some(allocator) = allocators.first() {
+ let agent_count = <T::Torus>::agent_ids().count() as u64;
+ // Read all agents
+ weight = weight.saturating_add(T::DbWeight::get().reads(agent_count));
+
for agent in <T::Torus>::agent_ids() {
if allocators.contains(&agent) {
continue;
}
if let Err(err) =
<Pallet<T> as Emission0Api<T::AccountId>>::delegate_weight_control(
&agent, allocator,
)
{
polkadot_sdk::sp_tracing::error!(
"failed to delegate weight control from {agent:?} to {allocator:?}: {err:?}"
);
+ } else {
+ // Successful delegation involves storage writes
+ weight = weight.saturating_add(T::DbWeight::get().writes(1));
}
}
} else {
polkadot_sdk::sp_tracing::error!("no allocators available");
}
- Weight::zero()
+ weight
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Weight::zero() | |
| fn on_runtime_upgrade() -> Weight { | |
| // Initialize weight tracking | |
| let mut weight = Weight::zero(); | |
| // Account for initial storage read of allocators | |
| weight = weight.saturating_add(T::DbWeight::get().reads(1)); | |
| let allocators: BTreeSet<_> = <T::Governance>::get_allocators().collect(); | |
| if let Some(allocator) = allocators.first() { | |
| let agent_count = <T::Torus>::agent_ids().count() as u64; | |
| // Account for reading all agent IDs | |
| weight = weight.saturating_add(T::DbWeight::get().reads(agent_count)); | |
| for agent in <T::Torus>::agent_ids() { | |
| if allocators.contains(&agent) { | |
| continue; | |
| } | |
| if let Err(err) = | |
| <Pallet<T> as Emission0Api<T::AccountId>>::delegate_weight_control( | |
| &agent, | |
| allocator, | |
| ) | |
| { | |
| polkadot_sdk::sp_tracing::error!( | |
| "failed to delegate weight control from {agent:?} to {allocator:?}: {err:?}" | |
| ); | |
| } else { | |
| // Each successful delegation writes to storage once | |
| weight = weight.saturating_add(T::DbWeight::get().writes(1)); | |
| } | |
| } | |
| } else { | |
| polkadot_sdk::sp_tracing::error!("no allocators available"); | |
| } | |
| // Return the accumulated weight instead of zero | |
| weight | |
| } |
🤖 Prompt for AI Agents
In pallets/emission0/src/migrations.rs at line 42, the migration function
currently returns Weight::zero() despite performing multiple storage reads and
writes. Update the code to calculate and return the actual weight by estimating
the number of agents processed and accounting for the associated storage
operations, ensuring the weight reflects the true computational cost of the
migration.
| let threshold: BalanceOf<T> = 50_000_000_000_000_000_000_000u128 | ||
| .try_into() | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make faucet threshold configurable
The threshold value is hardcoded to 50,000 tokens. Consider making this a configurable parameter in the pallet's Config trait for better flexibility.
Add to the Config trait:
/// Maximum total balance (free + staked) for faucet eligibility
type FaucetThreshold: Get<BalanceOf<Self>>;Then use it:
- let threshold: BalanceOf<T> = 50_000_000_000_000_000_000_000u128
- .try_into()
- .unwrap_or_default();
+ let threshold = T::FaucetThreshold::get();🤖 Prompt for AI Agents
In pallets/faucet/src/lib.rs around lines 137 to 139, the faucet threshold is
hardcoded to a fixed value, limiting flexibility. To fix this, add a new
associated type FaucetThreshold to the pallet's Config trait with the bound
Get<BalanceOf<Self>>. Then replace the hardcoded threshold assignment with a
call to T::FaucetThreshold::get() to retrieve the configurable threshold value.
| .filter_map(|id| { | ||
| AccumulatedStreamAmounts::<T>::get((&contract.grantor, id, permission_id)) | ||
| }) | ||
| .fold(BalanceOf::<T>::zero(), |acc, e| acc.saturating_add(e)), // The Balance AST does not enforce the Sum trait |
There was a problem hiding this comment.
Use checked arithmetic for financial calculations.
Using saturating_add for accumulating balances could hide overflow issues. For financial calculations, consider using checked arithmetic to explicitly handle overflows.
- .fold(BalanceOf::<T>::zero(), |acc, e| acc.saturating_add(e))
+ .try_fold(BalanceOf::<T>::zero(), |acc, e| acc.checked_add(&e).ok_or(Error::<T>::Overflow))?Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In pallets/permission0/src/ext/emission_impl.rs at line 240, replace the use of
saturating_add with checked_add when accumulating balances to explicitly handle
potential overflow errors. Update the fold operation to use checked_add and
handle the Option result properly, such as by returning an error or defaulting
safely, to ensure overflow issues are not silently ignored in financial
calculations.
|
|
||
| /// Create a new namespace, automatically creating missing intermediate nodes | ||
| #[pallet::call_index(7)] | ||
| #[pallet::weight(Weight::default())] |
There was a problem hiding this comment.
Add proper weight calculations for namespace operations.
Both create_namespace and delete_namespace use Weight::default() which is incorrect for operations that perform storage reads and writes. This could lead to incorrect fee calculations and potential DoS vectors.
Implement proper weight calculations based on the storage operations performed.
Also applies to: 385-385
🤖 Prompt for AI Agents
In pallets/torus0/src/lib.rs at lines 368 and 385, the weight annotations for
create_namespace and delete_namespace use Weight::default(), which does not
reflect the actual storage reads and writes these functions perform. Replace
Weight::default() with calculated weights that account for the number and type
of storage operations involved, ensuring accurate fee calculation and preventing
potential DoS attacks.
| fn set_allocator(key: &T::AccountId) { | ||
| crate::Allocators::<T>::insert(key, ()); | ||
| Allocators::<T>::insert(key, ()); | ||
| } |
There was a problem hiding this comment.
Add validation or restrict visibility of set_allocator.
The set_allocator method directly inserts into storage without validation. Ensure this is only called from validated contexts or add appropriate checks.
Consider making this method internal or adding validation to prevent arbitrary allocator additions.
🤖 Prompt for AI Agents
In pallets/governance/src/lib.rs around lines 585 to 587, the set_allocator
function inserts directly into storage without any validation or access control.
To fix this, restrict the visibility of set_allocator by making it a private or
internal function, or add validation logic to ensure it is only called from
authorized or validated contexts, preventing arbitrary allocator additions.
| if n > 0 { | ||
| let mut base = I92F36::from_num(1.5); | ||
| let mut result = I92F36::from_num(1); | ||
| let mut base = FixedU128::from_inner((1.5 * FixedU128::DIV as f64) as u128); |
There was a problem hiding this comment.
Avoid floating-point arithmetic in consensus-critical code
Using floating-point arithmetic (1.5 * FixedU128::DIV as f64) can lead to non-determinism across different architectures, potentially causing consensus failures.
Replace the floating-point calculation with fixed-point arithmetic:
- let mut base = FixedU128::from_inner((1.5 * FixedU128::DIV as f64) as u128);
+ // Use 3/2 = 1.5 in fixed-point representation
+ let mut base = FixedU128::from_rational(3u128, 2u128);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let mut base = FixedU128::from_inner((1.5 * FixedU128::DIV as f64) as u128); | |
| // Use 3/2 = 1.5 in fixed-point representation | |
| let mut base = FixedU128::from_rational(3u128, 2u128); |
🤖 Prompt for AI Agents
In pallets/governance/src/proposal.rs at line 679, replace the floating-point
arithmetic used to calculate the fixed-point value with purely integer-based
fixed-point arithmetic to avoid non-determinism. Instead of multiplying 1.5 by
FixedU128::DIV as a float, compute the equivalent fixed-point value using
integer operations, such as multiplying FixedU128::DIV by 3 and then dividing by
2, to maintain determinism in consensus-critical code.
| match self.data { | ||
| ProposalData::Emission { .. } => self.creation_block.saturating_add(21_600), | ||
| ProposalData::Emission { .. } => self.creation_block.saturating_add( | ||
| U256::from(21_600) | ||
| .try_into() | ||
| .ok() | ||
| .expect("this is a safe conversion"), | ||
| ), | ||
| _ => self.expiration_block, |
There was a problem hiding this comment.
Potential panic in block number conversion
The conversion from U256 to BlockNumberFor<T> uses expect() which will panic if the conversion fails. While 21,600 is a safe value, consider using a more defensive approach.
- ProposalData::Emission { .. } => self.creation_block.saturating_add(
- U256::from(21_600)
- .try_into()
- .ok()
- .expect("this is a safe conversion"),
- ),
+ ProposalData::Emission { .. } => {
+ // 21,600 blocks ≈ 2 days at 8 seconds per block
+ let emission_delay = BlockNumberFor::<T>::from(21_600u32);
+ self.creation_block.saturating_add(emission_delay)
+ },📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| match self.data { | |
| ProposalData::Emission { .. } => self.creation_block.saturating_add(21_600), | |
| ProposalData::Emission { .. } => self.creation_block.saturating_add( | |
| U256::from(21_600) | |
| .try_into() | |
| .ok() | |
| .expect("this is a safe conversion"), | |
| ), | |
| match self.data { | |
| ProposalData::Emission { .. } => { | |
| // 21,600 blocks ≈ 2 days at 8 seconds per block | |
| let emission_delay = BlockNumberFor::<T>::from(21_600u32); | |
| self.creation_block.saturating_add(emission_delay) | |
| }, | |
| // … other match arms … | |
| } |
🤖 Prompt for AI Agents
In pallets/governance/src/proposal.rs around lines 52 to 58, the conversion from
U256 to BlockNumberFor<T> uses expect(), which can cause a panic if the
conversion fails. Replace expect() with a safer method such as handling the
Result with match or unwrap_or to provide a default or error handling path,
ensuring the code does not panic even if the conversion unexpectedly fails.
saiintbrisson
force-pushed
the
dev
branch
3 times, most recently
from
957e3d8 to
612a6e0
Compare
saiintbrisson
force-pushed
the
dev
branch
2 times, most recently
from
3d19852 to
a768e6f
Compare
Context
In blockchain development where code governs real economic value, we need clear git practices that prioritize traceability and auditability over short-term convenience. This documentation establishes our preference for merge commits to preserve full commit history for incidents, audits, and upgrades.
Key Changes
Rationale
Moving away from squash merges to preserve:
git bisect,git blame, and code auditsSummary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores
Tests
Documentation