Implementing Nolus lending library #369
Conversation
WalkthroughA new lending library for the Nolus protocol was added, including its contract, configuration, messages, schema, and utility types. The workspace and program manager were updated to integrate this library, and compatibility types were introduced in the lending-utils package. The program manager's schema and configuration enums were extended to support the new library. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ProgramManager
participant NolusLendingLibrary
participant NolusPool
User->>ProgramManager: Configure ValenceNolusLending library
ProgramManager->>NolusLendingLibrary: Instantiate with LibraryConfig
User->>NolusLendingLibrary: Execute Lend or Withdraw
NolusLendingLibrary->>NolusPool: Deposit or Burn (on behalf of input_addr)
NolusPool-->>NolusLendingLibrary: Reply with result
NolusLendingLibrary->>NolusLendingLibrary: Handle reply (for Withdraw)
NolusLendingLibrary->>NolusPool: Query balance
NolusLendingLibrary->>OutputAccount: Send withdrawn funds (if any)
Possibly related PRs
Suggested reviewers
Poem
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (13)
packages/lending-utils/src/lib.rs (1)
2-2: New Nolus Module Integration
Introducing thenolusmodule aligns with the new compatibility types. Ensuresrc/nolus.rsexists and includes module-level documentation for public API clarity.contracts/libraries/nolus-lending/src/lib.rs (1)
1-2: Module Entry Points Added
Definingcontractandmsgmodules establishes the library’s structure. Ensure both modules include appropriate documentation and tests for their exported types.contracts/libraries/nolus-lending/README.md (2)
1-4: Consistency and grammar in README introduction
- Consider renaming the title to “Valence Nolus Lending Library” to match naming conventions of other libraries.
- Change “withdrawed tokens” to “withdrawn tokens” for correct grammar.
9-20: Align Rust doc comment style and formatting
- Use
///for all field documentation instead of mixing//and///.- Ensure code fences specify the language (
rust) and include blank lines before and after blocks for readability.contracts/libraries/nolus-lending/src/bin/schema.rs (1)
6-12: Improve macro formatting
Add a space after commas in generic parameters for readability:-execute: ExecuteMsg<FunctionMsgs,LibraryConfigUpdate>, +execute: ExecuteMsg<FunctionMsgs, LibraryConfigUpdate>,packages/lending-utils/src/nolus.rs (1)
8-18: Consider using unit variant style for clarity
TheDeposit()variant is currently defined with parentheses; you can simplify it to a unit variant (Deposit) to match Rust conventions and improve readability.Example diff:
- Deposit(), + Deposit,contracts/libraries/nolus-lending/src/msg.rs (3)
26-31: Use consistent documentation comment style.The comments should use
///for documentation consistency.- // Address of the input account (Valence interchain account) + /// Address of the input account (Valence interchain account) pub input_addr: LibraryAccountType, /// Address of the output account pub output_addr: LibraryAccountType, - // Address of the pool contract + /// Address of the pool contract pub pool_addr: String, - // Denom of the asset we are going to lend + /// Denom of the asset we are going to lend pub denom: String,
71-74: Remove redundant field assignment.The
pool_addrfield assignment uses redundant syntax.Ok(Config { input_addr, output_addr, - pool_addr: pool_addr, + pool_addr, denom: self.denom.clone(), })
121-122: Remove redundant field assignment.The
pool_addrfield assignment uses redundant syntax.input_addr, output_addr, - pool_addr: pool_addr, + pool_addr, denom,contracts/libraries/nolus-lending/src/contract.rs (4)
72-73: Improve attribute value for better clarity.The balance attribute includes both amount and denom (e.g., "1000uatom"). Consider using just the amount for clearer logs.
.add_attribute("method", "lend") - .add_attribute("amount", balance.to_string()) + .add_attribute("amount", balance.amount.to_string()) + .add_attribute("denom", balance.denom) .add_attribute("input_addr", cfg.input_addr.to_string()))
96-96: Remove unnecessary type conversion.The
balance_nlpis already of typeUint128, so calling.into()is redundant.- if amt > balance_nlp.into() || amt.is_zero() { + if amt > balance_nlp || amt.is_zero() {
117-121: Consider simplifying the reply data handling.The configuration is already available in the contract state. Instead of serializing it and passing through the payload, you could load it from storage in the reply handler.
// Execute on behalf of input_addr with reply. On reply we will send the funds to the output address let execute_msg = execute_submsgs_on_behalf_of( vec![SubMsg::reply_on_success(withdraw_msg, WITHDRAW_REPLY_ID)], - Some(to_json_string(&cfg)?), + None, &cfg.input_addr, )?;Then in the reply handler, load the config from storage:
WITHDRAW_REPLY_ID => { - // Extract configuration from the reply payload - let cfg: Config = valence_account_utils::msg::parse_valence_payload(&msg.result)?; + // Load configuration from storage + let cfg: Config = valence_library_base::load_config(deps.storage)?;
158-160: Improve attribute value for better clarity.Similar to the lend method, consider separating amount and denom in attributes.
.add_attribute("method", "withdraw") - .add_attribute("amount", balance.to_string()) + .add_attribute("amount", balance.amount.to_string()) + .add_attribute("denom", balance.denom) .add_attribute("output_addr", cfg.output_addr.to_string()))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
Cargo.toml(2 hunks)contracts/libraries/nolus-lending/.cargo/config.toml(1 hunks)contracts/libraries/nolus-lending/Cargo.toml(1 hunks)contracts/libraries/nolus-lending/README.md(1 hunks)contracts/libraries/nolus-lending/schema/valence-nolus-lending.json(1 hunks)contracts/libraries/nolus-lending/src/bin/schema.rs(1 hunks)contracts/libraries/nolus-lending/src/contract.rs(1 hunks)contracts/libraries/nolus-lending/src/lib.rs(1 hunks)contracts/libraries/nolus-lending/src/msg.rs(1 hunks)packages/lending-utils/Cargo.toml(1 hunks)packages/lending-utils/src/lib.rs(1 hunks)packages/lending-utils/src/nolus.rs(1 hunks)program-manager/Cargo.toml(1 hunks)program-manager/schema/valence-program-manager.json(4 hunks)program-manager/src/library.rs(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
contracts/libraries/nolus-lending/src/bin/schema.rs (1)
contracts/libraries/nolus-lending/src/contract.rs (3)
instantiate(22-29)execute(32-39)query(176-194)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: setup-local-ic
- GitHub Check: solidity contracts
- GitHub Check: build-and-deploy
🔇 Additional comments (15)
packages/lending-utils/Cargo.toml (1)
11-11: Dependency Addition Approved
Addingserdetolending-utilsis appropriate for enablingSerialize/Deserializederives.contracts/libraries/nolus-lending/.cargo/config.toml (1)
1-3: Cargo Alias Setup Looks Good
Thewasmandschemaaliases streamline build and schema generation workflows.program-manager/src/library.rs (1)
101-101:✅ Verification successful
Register Nolus Lending in LibraryConfig
AddingValenceNolusLendingto the enum is correct. Please verify that the dependency is declared inprogram-manager/Cargo.tomland that the JSON schema (program-manager/schema/valence-program-manager.json) has been updated accordingly.
🏁 Script executed:
#!/bin/bash set -e rg -n 'valence-nolus-lending' program-manager/Cargo.toml rg -n 'ValenceNolusLending' program-manager/schema/valence-program-manager.jsonLength of output: 376
✅ Dependency and schema updates verified
valence-nolus-lendingis declared in program-manager/Cargo.toml (line 48)ValenceNolusLendingentries exist in the JSON schema (lines 933, 936, 1498, 1501)LGTM!
program-manager/Cargo.toml (1)
48-48: Add valence-nolus-lending dependency
New workspace dependency added for the Nolus lending library, aligning with the broader integration in this PR.Cargo.toml (1)
103-103: Add Nolus lending middleware package
Workspace dependencyvalence-nolus-lendinghas been included under middleware with the"library"feature, consistent with existing lending libraries.packages/lending-utils/src/nolus.rs (3)
5-6: Dependencies are correctly imported
The required typesAddrandUint128fromcosmwasm_std, along withSerialize/Deserializefromserde, are in place.Please confirm that this module is publicly re-exported in
packages/lending-utils/src/lib.rsso consumers can uselending_utils::nolus::*.
20-29: QueryMsg enum is well-defined
TheBalance { address: Addr }variant accurately captures the balance query for the CW20 interface. Serialization attributes enforce snake_case and strict field checks.
32-37: BalanceResponse struct aligns with schema
The response struct cleanly wraps thebalancefield with appropriate serde annotations. Looks good.program-manager/schema/valence-program-manager.json (3)
929-941: Approve newValenceNolusLendingvariant integration
The schema’sLibraryConfigunion now correctly includes theValenceNolusLendingvariant referring toLibraryConfig15. This aligns with the new lending library addition.
1069-1097: LibraryConfig15 definition matches expected shape
The newLibraryConfig15mirrors other library configs (denom, input_addr, output_addr, pool_addr) and enforcesadditionalProperties: false. Schema is consistent and correct.
1707-1744: LibraryConfigUpdate15 definition aligns with update patterns
TheLibraryConfigUpdate15object allows partial updates to the Nolus lending config in line with otherLibraryConfigUpdateXdefinitions. This is consistent and ready for use.contracts/libraries/nolus-lending/src/msg.rs (2)
8-14: LGTM!The
FunctionMsgsenum is well-structured with clear documentation for lending and withdrawal operations.
16-21: LGTM!The
QueryMsgenum is properly set up with the necessary macros for a Valence library contract.contracts/libraries/nolus-lending/src/contract.rs (2)
21-29: LGTM!The instantiate function correctly delegates to the base library implementation with proper versioning.
49-128: Well-structured lending and withdrawal logic!The implementation correctly handles:
- Balance validation before lending
- Proper error handling for zero balances
- Flexible withdrawal amounts (full or partial)
- Appropriate use of submessages for withdrawal flow
| "LibraryAccountType": { | ||
| "description": "A helper type that is used to associate an account or library with an id When a program is not instantiated yet, ids will be used to reference accounts and libraries When a program is instantiated, the ids will be replaced by the instantiated addresses", | ||
| "oneOf": [ | ||
| { | ||
| "type": "object", | ||
| "required": [ | ||
| "|library_account_addr|" | ||
| ], | ||
| "properties": { | ||
| "|library_account_addr|": { | ||
| "type": "string" | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| }, | ||
| { | ||
| "type": "object", | ||
| "required": [ | ||
| "|account_id|" | ||
| ], | ||
| "properties": { | ||
| "|account_id|": { | ||
| "type": "integer", | ||
| "format": "uint64", | ||
| "minimum": 0.0 | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| }, | ||
| { | ||
| "type": "object", | ||
| "required": [ | ||
| "|library_id|" | ||
| ], | ||
| "properties": { | ||
| "|library_id|": { | ||
| "type": "integer", | ||
| "format": "uint64", | ||
| "minimum": 0.0 | ||
| } | ||
| }, | ||
| "additionalProperties": false | ||
| } | ||
| ] |
There was a problem hiding this comment.
Fix placeholder property names in LibraryAccountType
The keys |library_account_addr|, |account_id|, and |library_id| appear to be placeholder tokens and must be replaced with valid identifiers (library_account_addr, account_id, library_id), otherwise the JSON schema will be invalid.
Suggested patch:
--- a/contracts/libraries/nolus-lending/schema/valence-nolus-lending.json
+++ b/contracts/libraries/nolus-lending/schema/valence-nolus-lending.json
@@ -32,21 +32,21 @@
"LibraryAccountType": {
"description": "A helper type ...",
"oneOf": [
{
"type": "object",
"required": [
- "|library_account_addr|"
+ "library_account_addr"
],
"properties": {
- "|library_account_addr|": {
+ "library_account_addr": {
"type": "string"
}
},
"additionalProperties": false
},
{
"type": "object",
"required": [
- "|account_id|"
+ "account_id"
],
"properties": {
- "|account_id|": {
+ "account_id": {
"type": "integer",
"format": "uint64",Repeat similarly for the library_id case.
📝 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.
| "LibraryAccountType": { | |
| "description": "A helper type that is used to associate an account or library with an id When a program is not instantiated yet, ids will be used to reference accounts and libraries When a program is instantiated, the ids will be replaced by the instantiated addresses", | |
| "oneOf": [ | |
| { | |
| "type": "object", | |
| "required": [ | |
| "|library_account_addr|" | |
| ], | |
| "properties": { | |
| "|library_account_addr|": { | |
| "type": "string" | |
| } | |
| }, | |
| "additionalProperties": false | |
| }, | |
| { | |
| "type": "object", | |
| "required": [ | |
| "|account_id|" | |
| ], | |
| "properties": { | |
| "|account_id|": { | |
| "type": "integer", | |
| "format": "uint64", | |
| "minimum": 0.0 | |
| } | |
| }, | |
| "additionalProperties": false | |
| }, | |
| { | |
| "type": "object", | |
| "required": [ | |
| "|library_id|" | |
| ], | |
| "properties": { | |
| "|library_id|": { | |
| "type": "integer", | |
| "format": "uint64", | |
| "minimum": 0.0 | |
| } | |
| }, | |
| "additionalProperties": false | |
| } | |
| ] | |
| "LibraryAccountType": { | |
| "description": "A helper type that is used to associate an account or library with an id When a program is not instantiated yet, ids will be used to reference accounts and libraries When a program is instantiated, the ids will be replaced by the instantiated addresses", | |
| "oneOf": [ | |
| { | |
| "type": "object", | |
| "required": [ | |
| "library_account_addr" | |
| ], | |
| "properties": { | |
| "library_account_addr": { | |
| "type": "string" | |
| } | |
| }, | |
| "additionalProperties": false | |
| }, | |
| { | |
| "type": "object", | |
| "required": [ | |
| "account_id" | |
| ], | |
| "properties": { | |
| "account_id": { | |
| "type": "integer", | |
| "format": "uint64", | |
| "minimum": 0.0 | |
| } | |
| }, | |
| "additionalProperties": false | |
| }, | |
| { | |
| "type": "object", | |
| "required": [ | |
| "library_id" | |
| ], | |
| "properties": { | |
| "library_id": { | |
| "type": "integer", | |
| "format": "uint64", | |
| "minimum": 0.0 | |
| } | |
| }, | |
| "additionalProperties": false | |
| } | |
| ] | |
| }, |
🤖 Prompt for AI Agents
In contracts/libraries/nolus-lending/schema/valence-nolus-lending.json between
lines 27 and 70, the property names in the LibraryAccountType schema use
placeholder tokens with pipe characters (e.g., "|library_account_addr|").
Replace these placeholder keys with valid identifiers by removing the pipe
characters, changing "|library_account_addr|" to "library_account_addr",
"|account_id|" to "account_id", and "|library_id|" to "library_id" throughout
the required and properties fields to ensure the JSON schema is valid.
stana-miric
force-pushed
the
smiric/nolus-lending
branch
from
a32f035 to
2838618
Compare
stana-miric
force-pushed
the
smiric/nolus-lending
branch
from
2838618 to
b52a909
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/src/libraries/cosmwasm/nolus_lending.md (3)
1-1: Use consistent title casing for the header.The document title should capitalize “Library” to match the style used elsewhere (e.g., in the Mermaid diagram title).
5-6: Hyphenate the compound adjective.Change “High Level Flow” to “High-Level Flow” to follow standard English usage for compound modifiers.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ns directly from the input account. ## High Level Flow ```mermaid --- title: Nolus Lendi...(EN_COMPOUND_ADJECTIVE_INTERNAL)
39-46: Unify Rust comment style in code snippet.Use
///for all struct field comments so they generate documentation consistently.38~ pub struct LibraryConfig { -39~ // Address of the input account +39~ /// Address of the input account 40~ pub input_addr: LibraryAccountType, -41~ /// Address of the output account +41~ /// Address of the output account -42~ // Address of the pool contract +42~ /// Address of the pool contract 43~ pub pool_addr: String, -44~ // Denom of the asset we are going to lend +44~ /// Denom of the asset we are going to lend 45~ pub denom: String, 46~ }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
docs/src/SUMMARY.md(1 hunks)docs/src/libraries/cosmwasm/nolus_lending.md(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- docs/src/SUMMARY.md
🧰 Additional context used
🪛 LanguageTool
docs/src/libraries/cosmwasm/nolus_lending.md
[uncategorized] ~5-~5: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...ns directly from the input account. ## High Level Flow ```mermaid --- title: Nolus Lendi...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
[grammar] ~60-~60: Using ‘plenty’ without ‘of’ is considered to be informal.
Context: ... a positive amount of nLPN tokens. 2. Amount Calculation: Uses the exact amount if specified; ...
(PLENTY_OF_NOUNS)
🔇 Additional comments (1)
docs/src/libraries/cosmwasm/nolus_lending.md (1)
7-24:⚠️ Potential issueClose the Mermaid code block.
The Mermaid snippet isn’t terminated with a closing triple backtick, which will break rendering.
Apply this diff after line 24:
+ ``` # <-- add this to close the mermaid blockLikely an incorrect or invalid review comment.
keyleu
left a comment
keyleu
left a comment
There was a problem hiding this comment.
LGTM, except for a nit in docs part. Maybe let's wait to merge to see if
- We're allowed to deploy on Nolus
- If we deploy, we can test it.
wdyt?
| P -- 1/Lend or Withdraw --> S | ||
| S -- 2/Query balances --> IA | ||
| S -- 3/Execute Lending --> IA | ||
| S -- 5/Execute Withdrawal --> IA | ||
| IA -- 6/Transfer Assets --> OA |
There was a problem hiding this comment.
| P -- 1/Lend or Withdraw --> S | |
| S -- 2/Query balances --> IA | |
| S -- 3/Execute Lending --> IA | |
| S -- 5/Execute Withdrawal --> IA | |
| IA -- 6/Transfer Assets --> OA | |
| P -- 1/Lend or Withdraw --> S | |
| S -- 2/Query balances --> IA | |
| S -- 3/Execute Lending --> IA | |
| S -- 4/Execute Withdrawal --> IA | |
| IA -- 5/Transfer Assets --> OA |
There was a problem hiding this comment.
Sure, it seems that we wont be allowed to deploy and that some changes will be needed. I will change the status of this PR to draft and then we will see what will happen with it. I'll fix the above mentioned things.
2 participants
This PR adds implementation for Nolus lending library.
*NOTE: since the contracts are not yet deployed on Nolus network, the library is not tested
Summary by CodeRabbit
New Features
Documentation
Chores