docs: updated confidential resource builder comments #1548
Conversation
WalkthroughConfidentialResourceBuilder expanded with new fields and many chainable configuration methods; introduces two build flows—build() (no mint) and initial_supply(ConfidentialOutputStatement) which mints confidential initial supply returning a Bucket. Adds view-key visibility, token symbol, divisibility, metadata, access/owner rules, authorization hooks, and optional address allocation. ResourceManager docs updated. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Dev as Developer
participant CB as ConfidentialResourceBuilder
participant RM as ResourceManager
participant RA as ResourceAddress
participant B as Bucket
rect rgb(238,245,255)
note right of Dev: Configure builder via chainable methods\n(e.g., with_view_key, with_token_symbol,\nwith_divisibility, access rules, auth hook, etc.)
Dev->>CB: new() + with_*(...)
end
alt Build without initial mint
Dev->>CB: build()
CB->>RM: create(metadata, rules, owner, alloc, hook, divisibility, view_key, symbol)
RM-->>CB: ResourceAddress
CB-->>Dev: ResourceAddress
else Build with initial supply
Dev->>CB: initial_supply(ConfidentialOutputStatement)
CB->>RM: create(..., MintArg::Confidential)
RM-->>CB: (ResourceAddress, Bucket)
CB-->>Dev: Bucket (initial confidential supply)
note over B,Dev: Bucket contains minted confidential supply
end
note over CB,RM: Authorization hook (if set) may be referenced at runtime
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
crates/template_lib/src/resource/manager.rs (2)
268-276: Doc correctness: wrong resource type and misleading example inmint_stealth
- The panic condition mentions
ResourceType::Confidential, but this API is for stealth resources.- The example assigns the return to
bucket, but the function returns().These are doc correctness issues that can mislead users.
Apply this diff:
- /// - The resource is not of type [`ResourceType::Confidential`] + /// - The resource is not of type [`ResourceType::Stealth`]- /// let bucket = resource_manager.mint_stealth(statement); + /// resource_manager.mint_stealth(statement);
669-672: Example usesengineinstead ofresource_managerThe example under
recall_confidentialreferences a non-existentenginevariable. Useresource_managerfor consistency with other examples.Apply this diff:
- /// let bucket = engine.recall_confidential(vault_id, commitments, revealed_amount); + /// let bucket = resource_manager.recall_confidential(vault_id, commitments, revealed_amount);
🧹 Nitpick comments (7)
crates/template_lib/src/resource/manager.rs (2)
133-136: Fix typos in Note and clarify internal visibilityMinor grammar and spelling issues in the new Note. Also clarify that it’s not publicly callable due to
pub(crate)visibility.Apply this diff:
- /// # Note - /// - /// This in an internal function that is used by the `ResourceBuilder` to create resources. You cannot publically call it. - /// + /// # Note + /// + /// This is an internal function used by the `ResourceBuilder` to create resources. It is not publicly callable.
741-744: Error message nit: decoding Option typeThe error string in
total_supply_optsuggests decoding anAmount, but the type isOption<Amount>. Adjust the message for accuracy.Apply this diff:
- resp.decode().expect("[total_supply] Failed to decode Amount") + resp.decode().expect("[total_supply_opt] Failed to decode Option<Amount>")crates/template_lib/src/resource/builder/confidential.rs (5)
14-58: Polish top-level builder docs: fix bracket typo and clarify Note
- Fix a stray closing
]inConfidentialOutputStatement.- Slightly improve phrasing for clarity.
Apply this diff:
-/// The `ConfidentialResourceBuilder` requires you to provide a [`ConfidentialOutputStatement`] if you wish to set an initial supply. -/// Additional supply can be minted later using the `mintable` access rule via another provided [`ConfidentialOutputStatement`]. Generation -/// of the `ConfidentialOutputStatement`] is non-trivial; examples of creation of the statement can be found in the test cases for the -/// `ConfidentialResourceBuilder`. +/// The `ConfidentialResourceBuilder` requires you to provide a [`ConfidentialOutputStatement`] if you wish to set an initial supply. +/// Additional supply can be minted later using the `mintable` access rule via another [`ConfidentialOutputStatement`]. +/// Generating a [`ConfidentialOutputStatement`] is non-trivial; examples of constructing the statement can be found +/// in the test cases for the `ConfidentialResourceBuilder`.
129-137: Rustdoc header casing: “# Examples”Use the conventional “# Examples” header in Rustdoc.
Apply this diff:
- /// #Examples + /// # Examples
149-156: Rustdoc header casing: “# Examples”Same header nit in the
burnablesection.Apply this diff:
- /// #Examples + /// # Examples
330-333: Grammar: “This method will panic if”Minor grammar fix for the Panics section.
Apply this diff:
- /// # Panics - /// method will panic if: + /// # Panics + /// This method will panic if:
111-118: Clarify immutability of the view keySlight rephrase to make the immutability constraint explicit.
Apply this diff:
- /// # Note - /// It is not currently possible to change the view key after the resource is created. + /// # Note + /// The view key is immutable; it cannot be changed after the resource is created.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
crates/template_lib/src/resource/builder/confidential.rs(5 hunks)crates/template_lib/src/resource/manager.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
crates/template_lib/src/resource/builder/confidential.rs (4)
crates/template_lib/src/resource/builder/fungible.rs (11)
with_owner_rule(80-83)mintable(116-119)burnable(135-138)withdrawable(183-186)depositable(203-206)update_access_rules(224-227)with_token_symbol(238-241)add_metadata(260-263)with_metadata(281-284)with_image_url(298-300)build(377-380)crates/engine_types/src/resource.rs (2)
view_key(106-108)metadata(183-185)bindings/src/types/RistrettoPublicKeyBytes.ts (1)
RistrettoPublicKeyBytes(6-6)crates/template_lib/src/auth/access_rules.rs (5)
mintable(272-275)burnable(278-281)withdrawable(297-300)depositable(303-306)update_access_rules(309-312)
🔇 Additional comments (2)
crates/template_lib/src/resource/builder/confidential.rs (2)
378-385: LGTM: build flow clearly describedThe build path doc is clear and matches the internal helper flow. No issues.
387-398: LGTM: initial_supply docs are actionableGood detail on what the
ConfidentialOutputStatementmust include and how to mint revealed-only outputs.
sdbondi
left a comment
sdbondi
left a comment
There was a problem hiding this comment.
Nice thanks! Just some minor nits
ghpbot-tari-project
added
the
P-conflicts
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
crates/template_lib/src/resource/builder/confidential.rs (3)
41-46: Nice: example now usesResourceBuilder::confidential().Resolves past confusion with
fungible().
111-118: Tighten the view-key sentence.Minor grammar polish.
-/// The view key is [`RistrettoPublicKeyBytes`] type, that is, the compressed public key representation of the secret view key. +/// The view key is of type [`RistrettoPublicKeyBytes`], i.e., the compressed public key representation of the secret view key.
162-179: Good clarification on recall semantics.The “ALL tokens” wording removes ambiguity.
🧹 Nitpick comments (10)
crates/template_lib/src/resource/builder/confidential.rs (10)
31-34: Fix broken Markdown link and sentence flow.Close the stray bracket and reflow the paragraph.
-/// The `ConfidentialResourceBuilder` requires you to provide a [`ConfidentialOutputStatement`] if you wish to set an initial supply. -/// Additional supply can be minted later using the `mintable` access rule via another provided [`ConfidentialOutputStatement`]. Generation -/// of the `ConfidentialOutputStatement`] is non-trivial; examples of creation of the statement can be found in the test cases for the -/// `ConfidentialResourceBuilder`. +/// The `ConfidentialResourceBuilder` requires a [`ConfidentialOutputStatement`] if you wish to set an initial supply. +/// Additional supply can be minted later using the `mintable` access rule via another [`ConfidentialOutputStatement`]. +/// Generation of the [`ConfidentialOutputStatement`] is non-trivial; examples can be found in the test cases for +/// the `ConfidentialResourceBuilder`.
39-46: Use a safer default thanallow_allin the basic example.Recommend
require_owner()to avoid copy-pasting permissive rules.-/// use tari_template_lib::resource::builder::ResourceBuilder; +/// use tari_template_lib::resource::builder::ResourceBuilder; +/// use tari_template_lib::auth::AccessRule; @@ -/// .mintable(rule!(allow_all)) +/// .mintable(AccessRule::require_owner())
129-137: Consistent heading + safer rule in example.-/// #Examples +/// # Examples @@ -/// .mintable(rule!(allow_all)) +/// .mintable(AccessRule::require_owner())
148-156: Consistent heading + safer rule in example.-/// #Examples +/// # Examples @@ -/// .burnable(rule!(allow_all)) +/// .burnable(AccessRule::require_owner())
190-204: Avoid implying amount-specificity for confidential flows.Remove the parenthetical to prevent confusion.
-/// Allows you to pass an [`AccessRule`] that defines who can withdraw tokens (via a specified amount) from a vault. +/// Allows you to pass an [`AccessRule`] that defines who can withdraw tokens from a vault.
210-224: Same here: drop amount phrasing for deposit.-/// Allows you to pass an [`AccessRule`] that defines who can deposit tokens (via a specified amount) into a vault. +/// Allows you to pass an [`AccessRule`] that defines who can deposit tokens into a vault.
199-204: Examples mixrule!(...)macro andAccessRule::...variants.Pick one style across docs for consistency (prefer the explicit
AccessRule::...APIs or the macro, but not both).
324-346: Polish the panic docs and heading.-/// # Panics -/// method will panic if: -/// * The divisibility is greater than 18. +/// # Panics +/// This method will panic if the divisibility is greater than 18.
378-382: Grammar fix and stronger intra-doc link.-/// Build the resource, with no initial supply and returns the `ResourceAddress` of the created resource. +/// Builds the resource with no initial supply and returns the `ResourceAddress` of the created resource. @@ -/// To create a resource with an initial supply, see [ResourceBuilder::initial_supply]. +/// To create a resource with an initial supply, see [`Self::initial_supply`].
391-397: Tighten wording; avoid “proof proving” and format range.-/// - A valid range proof proving the output values are in range [minimum_value_promise, 2^64) +/// - A valid range proof demonstrating the output values are in range `[minimum_value_promise, 2^64)` @@ -/// Use [`ConfidentialOutputStatement::mint_revealed()`] to mint **revealed funds only** (no commitments). +/// Use [`ConfidentialOutputStatement::mint_revealed()`] to mint **revealed funds only** (no commitments).Please confirm the method name
mint_revealed()and its path are correct.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
crates/template_lib/src/resource/builder/confidential.rs(5 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
crates/template_lib/src/resource/builder/confidential.rs (3)
crates/template_lib/src/resource/builder/fungible.rs (11)
with_owner_rule(80-83)mintable(116-119)burnable(135-138)withdrawable(183-186)depositable(203-206)update_access_rules(224-227)with_token_symbol(238-241)add_metadata(260-263)with_metadata(281-284)with_image_url(298-300)build(377-380)crates/engine_types/src/resource.rs (2)
view_key(106-108)metadata(183-185)crates/template_lib/src/auth/access_rules.rs (5)
mintable(272-275)burnable(278-281)withdrawable(297-300)depositable(303-306)update_access_rules(309-312)
🔇 Additional comments (1)
crates/template_lib/src/resource/builder/confidential.rs (1)
14-28: Good docs expansion and structure.Clear overview, defaults, and two-path build flow are well explained.
| /// Sets up who can update the access rules of the resource. | ||
| /// | ||
| /// Allows you to pass an [`AccessRule`] that defines who can update the access rules of the resource. | ||
| /// | ||
| /// By default, the ability to update access rules is denied for all users. If you want to allow the owner to update | ||
| /// the access rules, you can use `.update_access_rules(AccessRule::require_owner())` | ||
| /// | ||
| /// # Examples | ||
| /// | ||
| /// ```rust, ignore | ||
| /// use tari_template_lib::auth::AccessRule; | ||
| /// use tari_template_lib::resource::builder::ResourceBuilder; | ||
| /// ResourceBuilder::confidential() | ||
| /// .update_access_rules(AccessRule::require_owner()) | ||
| /// .build(); | ||
| /// ``` | ||
| pub fn update_access_rules(mut self, rule: AccessRule) -> Self { |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Docs contradict earlier statement about who may update access rules.
Above (Lines 85–90) says only the owner can update after creation; here it says default is denied for all. Please align.
Suggested wording if owner is allowed by default:
-/// By default, the ability to update access rules is denied for all users. If you want to allow the owner to update
-/// the access rules, you can use `.update_access_rules(AccessRule::require_owner())`
+/// By default, only the owner may update the access rules. This function lets you override that behavior, e.g.:
+/// `.update_access_rules(AccessRule::require_owner())`If instead default is deny-all, update the earlier owner text accordingly. To verify defaults, please run:
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Show DEFAULT_DIVISIBILITY value
rg -nP 'const\s+DEFAULT_DIVISIBILITY\s*:\s*u8\s*=\s*\d+' -n crates
# Inspect ResourceAccessRules::new() defaults
rg -nP 'impl\s+ResourceAccessRules\b' -n crates -C3
rg -nP 'pub\s+fn\s+new\s*\(' -n crates -C6 | sed -n '1,200p'Length of output: 15512
To inspect the actual defaults in ResourceAccessRules::new(), run:
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Print the implementation of ResourceAccessRules::new to see default rules
sed -n '220,300p' crates/template_lib/src/auth/access_rules.rsLength of output: 2923
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
sed -n '1,200p' crates/template_lib/src/resource/builder/confidential.rsLength of output: 7568
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Show the remainder of the confidential builder to inspect update_access_rules docs
sed -n '200,360p' crates/template_lib/src/resource/builder/confidential.rsLength of output: 6062
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
# Locate the build method in the confidential resource builder
grep -R "pub fn build" -n crates/template_lib/src/resource/builder/confidential.rs
# Print the build implementation to see how owner_rule and access_rules are applied
sed -n '360,440p' crates/template_lib/src/resource/builder/confidential.rsLength of output: 2942
Align update_access_rules docs with the owner-only default
The current comment implies owners must opt in to update rules, but by default the OwnerRule already grants them access. Update the docs to reflect that only the owner may update unless overridden. For example:
-/// Sets up who can update the access rules of the resource.
-///
-/// Allows you to pass an [`AccessRule`] that defines who can update the access rules of the resource.
-///
-/// By default, the ability to update access rules is denied for all users. If you want to allow the owner to update
-/// the access rules, you can use `.update_access_rules(AccessRule::require_owner())`
+/// Sets up who can update the access rules of the resource.
+///
+/// Allows you to pass an [`AccessRule`] that defines who else (in addition to the owner) may update the rules.
+///
+/// By default, only the resource owner (per [`with_owner_rule`]) may update the access rules.
/// To grant broader permissions, for example:
+/// ```ignore
+/// ResourceBuilder::confidential()
+/// .update_access_rules(AccessRule::allow_all())
+/// .build();
+/// ```🤖 Prompt for AI Agents
crates/template_lib/src/resource/builder/confidential.rs around lines 230 to
246: the doc comment for update_access_rules incorrectly implies owners must opt
in to update rules; change the wording to state that by default the OwnerRule
grants the owner permission to update access rules and that update_access_rules
lets callers override that (e.g., show an example calling
update_access_rules(AccessRule::allow_all()) to allow everyone), and update the
explanatory text and example accordingly so it reflects the owner-only default
rather than owner opt-in.
ghpbot-tari-project
removed
the
P-conflicts
3 participants
Description
Extending Confidential resource documentation.
Motivation and Context
Match style and detail of fungible
How Has This Been Tested?
What process can a PR reviewer use to test or verify this change?
Breaking Changes
Summary by CodeRabbit
New Features
Documentation