Skip to content

Conversation

stana-miric
Copy link
Contributor

@stana-miric stana-miric commented Jun 2, 2025

This PR adds implementation for Elys lending library.

*NOTE: since the contracts are not yet deployed on Elys network (permissioned cosmwasm), the library is not tested.

Summary by CodeRabbit

  • New Features

    • Introduced the Elys Lending library, enabling lending, withdrawal, and reward claiming functionalities for Elys pools.
    • Added support for configuring input/output accounts and pool selection within the new lending library.
    • Integrated Elys Lending as a selectable library type in the program manager, with full schema and configuration support.
  • Documentation

    • Added comprehensive documentation and schema files for the Elys Lending library, detailing configuration, execution, and query interfaces.
  • Chores

    • Updated dependency lists and workspace configuration to include the new Elys Lending library and related utilities.

Copy link
Contributor

coderabbitai bot commented Jun 2, 2025

Walkthrough

A new Elys lending library is introduced, including contract logic, configuration, schema, and integration into the workspace and program manager. The changes add message types, configuration validation, and query helpers for Elys lending, update workspace and dependency configurations, and extend the program manager to support the new lending library type.

Changes

File(s) Change Summary
Cargo.toml, program-manager/Cargo.toml Added valence-elys-lending as a workspace member and dependency; reformatted workspace list.
contracts/libraries/elys-lending/* (Cargo.toml, README.md, .cargo/config.toml, schema/, src/) Added new Elys lending library: contract logic, message types, schema, config, build scripts.
packages/lending-utils/Cargo.toml, packages/lending-utils/src/lib.rs Added elys module and dependencies (serde, schemars, prost) to lending-utils package.
packages/lending-utils/src/elys.rs Introduced Elys-specific messages, queries, response types, and query helper for CosmWasm 2.x.
program-manager/schema/valence-program-manager.json Extended schema to support new ValenceElysLending library config and update types.
program-manager/src/library.rs Added ValenceElysLending variant to LibraryConfig enum.
docs/src/SUMMARY.md, docs/src/libraries/cosmwasm/elys_lending.md Added documentation entry and detailed README for the Elys Lending library.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ProgramManager
    participant ElysLendingLibrary
    participant ElysChain

    User->>ProgramManager: Instantiate or update program (with ValenceElysLending config)
    ProgramManager->>ElysLendingLibrary: Passes config for validation/instantiation
    ElysLendingLibrary->>ElysChain: Query pool info, balances, etc.
    ElysChain-->>ElysLendingLibrary: Pool and account info
    ElysLendingLibrary-->>ProgramManager: Success or error response

    User->>ElysLendingLibrary: Execute lending action (lend/withdraw/claim)
    ElysLendingLibrary->>ElysChain: Submit MsgBond/MsgUnbond/MsgClaimRewards
    ElysChain-->>ElysLendingLibrary: Reply/result
    ElysLendingLibrary->>User: Confirmation and/or asset transfer
Loading

Possibly related PRs

Suggested reviewers

  • keyleu

Poem

In the warren of code, new lending hops in,
Elys pools await with a whiskered grin.
Borrow, withdraw, or claim what you’ve earned,
With schema and queries, every detail discerned.
From workspace to manager, all snug in the patch—
Another fine feature, a rabbit’s proud catch! 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 077bd9d and 50306fb.

📒 Files selected for processing (1)
  • contracts/libraries/elys-lending/src/contract.rs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • contracts/libraries/elys-lending/src/contract.rs
✨ Finishing Touches
  • 📝 Generate Docstrings

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need 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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 8

🧹 Nitpick comments (13)
program-manager/src/library.rs (1)

101-101: New ValenceElysLending variant added to LibraryConfig.
You’ve correctly introduced the ValenceElysLending(valence_elys_lending::msg::LibraryConfig) variant. Please verify that:

  1. The manager_impl_library_configs macro supports this new variant.
  2. The JSON schema (program-manager/schema/valence-program-manager.json) is updated to include this variant for instantiation and update messages.

Let me know if you need help updating the schema or macro usage.

contracts/libraries/elys-lending/.cargo/config.toml (1)

1-3: Define Cargo aliases for build and schema generation.
The wasm and schema aliases streamline the build and schema commands for the Elys lending library. Consider documenting these aliases in the project README for discoverability, but the definitions themselves are correct.

contracts/libraries/elys-lending/README.md (1)

3-4: Typo in documentation.
The word "withdrawed" should be corrected to "withdrawn" for proper grammar:

- depositing the withdrawed tokens into an **output account**.
+ depositing the withdrawn tokens into an **output account**.
contracts/libraries/elys-lending/src/bin/schema.rs (1)

8-10: Improve readability in macro invocation.
Add a space after the comma between generic parameters to align with Rust formatting conventions:

- execute: ExecuteMsg<FunctionMsgs,LibraryConfigUpdate>,
+ execute: ExecuteMsg<FunctionMsgs, LibraryConfigUpdate>,
contracts/libraries/elys-lending/Cargo.toml (1)

1-7: Enhance crate metadata.
Consider enriching the [package] section with description, homepage, and documentation fields to improve discoverability on crates.io and docs.rs:

 [package]
 name                 = "valence-elys-lending"
+description          = "Elys lending library for the Valence protocol"
+homepage             = "https://github.com/timewave-computer/valence-protocol"
+documentation        = "https://docs.rs/valence-elys-lending"
 authors.workspace    = true
 edition.workspace    = true
 license.workspace    = true
 version.workspace    = true
 repository.workspace = true
packages/lending-utils/src/elys.rs (2)

28-44: Consider the data types used in PoolResponse.

All numeric fields in PoolResponse (rates, ratios, totals, etc.) are defined as String. While this might be intentional for compatibility with the Elys protocol, consider documenting why strings are used instead of numeric types like Decimal or Uint128. This would help future maintainers understand the design decision.


17-44: Consider adding documentation for complex pool parameters.

The structs are properly defined, but the PoolResponse contains many financial parameters that would benefit from documentation comments explaining their purpose and expected formats.

+/// Response containing pool information
 pub struct QueryGetPoolResponse {
     pub pool: ::core::option::Option<PoolResponse>,
 }

+/// Detailed pool parameters and state
 pub struct PoolResponse {
+    /// The denomination used for deposits in this pool
     pub deposit_denom: String,
+    /// Current redemption rate as a string-encoded decimal
     pub redemption_rate: String,
     // ... add documentation for other financial parameters
     pub pool_id: u64,
     pub total_deposit: String,
     pub total_borrow: String,
     pub borrow_ratio: String,
     pub max_withdraw_ratio: String,
 }
contracts/libraries/elys-lending/src/msg.rs (2)

61-77: Use field shorthand syntax.

The validation implementation is correct, but you can simplify the struct initialization.

         Ok(Config {
             input_addr,
             output_addr,
-            pool_id: pool_id,
+            pool_id,
         })

108-116: Use field shorthand syntax in constructor.

         Config {
             input_addr,
             output_addr,
-            pool_id: pool_id,
+            pool_id,
         }
contracts/libraries/elys-lending/schema/valence-elys-lending.json (4)

72-106: Consider centralizing repeated definitions.
The LibraryConfig schema currently references local definitions for LibraryAccountType and Uint64, which are duplicated in multiple blocks. For maintainability, consider moving shared types (e.g., LibraryAccountType, Uint64) into a single, root-level definitions or $defs section, and updating all $ref pointers accordingly.


387-422: LibraryConfigUpdate could use a description.
While the partial update fields are correct, consider adding a brief "description" at the LibraryConfigUpdate schema level to explain its purpose and how null values are handled.


423-438: Deduplicate numeric wrapper definitions.
The Uint128 and Uint64 definitions are repeated here. Consider referencing the shared definitions from earlier or moving them to a common location to avoid redundancy.


505-725: Consolidate response definitions across schemas.
The response schemas for get_library_config, get_processor, get_raw_library_config, and ownership each include local definitions (Addr, Uint64, LibraryAccountType, Expiration, Timestamp) that are largely duplicated. For better maintainability and DRY compliance, centralize shared types in a root-level definitions section and update local $ref references.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b101dfd and 719df4c.

⛔ Files ignored due to path filters (1)
  • Cargo.lock is excluded by !**/*.lock
📒 Files selected for processing (15)
  • Cargo.toml (2 hunks)
  • contracts/libraries/elys-lending/.cargo/config.toml (1 hunks)
  • contracts/libraries/elys-lending/Cargo.toml (1 hunks)
  • contracts/libraries/elys-lending/README.md (1 hunks)
  • contracts/libraries/elys-lending/schema/valence-elys-lending.json (1 hunks)
  • contracts/libraries/elys-lending/src/bin/schema.rs (1 hunks)
  • contracts/libraries/elys-lending/src/contract.rs (1 hunks)
  • contracts/libraries/elys-lending/src/lib.rs (1 hunks)
  • contracts/libraries/elys-lending/src/msg.rs (1 hunks)
  • packages/lending-utils/Cargo.toml (1 hunks)
  • packages/lending-utils/src/elys.rs (1 hunks)
  • packages/lending-utils/src/lib.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 (2)
contracts/libraries/elys-lending/src/bin/schema.rs (1)
contracts/libraries/elys-lending/src/contract.rs (3)
  • instantiate (27-34)
  • execute (37-44)
  • query (248-266)
packages/lending-utils/src/elys.rs (2)
contracts/libraries/elys-lending/src/contract.rs (1)
  • query (248-266)
program-manager/src/error.rs (1)
  • generic_err (92-94)
⏰ 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 (25)
packages/lending-utils/Cargo.toml (1)

11-13: Dependencies added for Elys integration look correct.
The serde, schemars, and prost crates are appropriately introduced for JSON serialization, schema generation, and protobuf support in the new elys module. Using workspace versions for serde and schemars maintains consistency.

packages/lending-utils/src/lib.rs (1)

1-1: Expose the new elys module.
Adding pub mod elys; correctly makes the elys helper functions available in the lending-utils crate. Ensure that packages/lending-utils/src/elys.rs fully implements the necessary CosmWasm message and query types.

program-manager/Cargo.toml (1)

48-48: Add valence-elys-lending to workspace dependencies.
The new dependency aligns with other valence-* libraries and enables the program manager to build and reference the Elys lending library. Version specification as { workspace = true } matches the workspace pattern.

contracts/libraries/elys-lending/src/lib.rs (1)

1-2: Public module declarations look good.
The lib.rs correctly exposes the contract and msg modules, laying out the library’s core structure. Ensure that contract.rs and msg.rs include proper documentation comments for public types and functions to aid discoverability.

Cargo.toml (1)

102-103: Integrate new Elys lending library.
The valence-elys-lending dependency with the "library" feature is correctly added under [workspace.dependencies]. Verify that the path contracts/libraries/elys-lending is accurate and that the workspace build picks up this crate as expected.

packages/lending-utils/src/elys.rs (4)

6-16: LGTM!

The ElysQuery enum is properly structured with appropriate derives and implements the required CustomQuery trait.


55-109: Protobuf messages are well-defined.

The message structs are properly configured for protobuf serialization with appropriate prost attributes and serde aliases for field name compatibility.


1-4: LGTM! Clear documentation of compatibility rationale.

The file header effectively explains why these messages are being redefined for CosmWasm 2.x compatibility and references the source version.


6-15: LGTM! Proper CustomQuery implementation.

The ElysQuery enum is well-defined with appropriate derives and implements the CustomQuery trait correctly. The snake_case serde annotation ensures consistent JSON serialization.

program-manager/schema/valence-program-manager.json (1)

929-941: Schema additions are consistent and well-structured.

The new ValenceElysLending library configuration follows the established pattern with appropriate required fields for instantiation and optional fields for updates.

Also applies to: 1069-1103, 1500-1512, 1713-1748

contracts/libraries/elys-lending/src/msg.rs (3)

9-24: Message enums are well-defined.

The FunctionMsgs enum properly covers lending operations with clear documentation, and QueryMsg is correctly prepared for future query implementations.


26-59: LibraryConfig is properly structured.

The configuration struct has clear field documentation and appropriate validation logic for converting account types to addresses.


79-99: Update logic includes proper validation.

Good practice to validate the pool exists before updating the configuration. The update method correctly handles partial updates.

contracts/libraries/elys-lending/src/contract.rs (6)

1-25: Contract setup follows best practices.

Good use of constants for reply IDs and standard contract versioning from Cargo.toml.


26-44: Entry points properly delegate to base library.

The instantiate and execute functions correctly use the base library pattern with appropriate type parameters.


54-88: Lend implementation is correct.

The lending logic properly validates balance, constructs the bond message, and executes on behalf of the input address. The use of Stargate messages is appropriate for custom Cosmos SDK messages.


98-107: Verify the single locked coin assumption.

The code assumes only one coin will be locked for this pool, but takes the first coin without checking the denomination. This could lead to withdrawing the wrong asset if multiple coins are locked.

Consider adding validation to ensure the locked coin matches the expected denomination:

+            // Query the pool to get the expected deposit denom
+            let pool = valence_lending_utils::elys::query_pool(&deps, cfg.pool_id.u64())?;
+            
             let balance_locked: Coin = tokens_locked
                 .locked_committed
                 .into_iter()
-                .next() // Take the first coin
+                .find(|coin| coin.denom == pool.deposit_denom)
                 .ok_or_else(|| {
                     LibraryError::ExecutionError(
                         "No funds locked for the specified denom".to_string(),
                     )
                 })?;

175-236: Reply handlers are well-implemented.

The reply logic correctly handles post-execution transfers. Note that the CLAIM_REPLY_ID handler is currently unreachable due to the missing submessage in the ClaimRewards function.


238-266: Configuration and query functions follow standard patterns.

The update and query implementations correctly delegate to appropriate handlers and return expected responses.

contracts/libraries/elys-lending/schema/valence-elys-lending.json (6)

2-4: Verify version consistency with contract code.
The contract_version ("0.2.0") and idl_version ("1.0.0") should align with the version in your Cargo.toml, the schema generation script, and the Elys lending library code.

Can you confirm that 0.2.0 matches the current crate version and that the IDL version is correct?


5-24: Instantiate message schema looks correct.
The InstantiateMsg defines all required fields (config, owner, processor) with strict additionalProperties: false, and includes a valid draft-07 $schema.


107-111: Uint64 wrapper definition is solid.
The Uint64 type uses a string encoding with clear documentation and examples.


113-183: Execute message top-level variants are well-defined.
The ExecuteMsg oneOf block cleanly separates process_function, update_config, update_processor, and update_ownership variants, each with strict property checks and additionalProperties: false.


186-283: Action and Expiration definitions are comprehensive.
The Action union covers object and string-based ownership operations, and the Expiration oneOf variants correctly model block height, time, and never-expire cases.


441-500: Query message schema is clear and strict.
The QueryMsg oneOf variants for get_processor, get_library_config, get_raw_library_config, and ownership are correctly defined with no additional properties.

Comment on lines 111 to 122
pub fn query_pool(deps: &DepsMut<ElysQuery>, pool_id: u64) -> StdResult<PoolResponse> {
let query = ElysQuery::QueryGetPool { pool_id };
let query_pool_response: Option<QueryGetPoolResponse> = deps.querier.query(&query.into()).ok();

// Check if pool exists, otherwise return an error
let pool_response =
query_pool_response.ok_or_else(|| StdError::generic_err("failed to query pool"))?;
let pool = pool_response
.pool
.ok_or_else(|| StdError::generic_err("pool not found"))?;

Ok(pool)
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Improve error handling to preserve error details.

The current implementation swallows error details and provides generic error messages. This makes debugging difficult when queries fail.

Consider preserving the original error information:

 pub fn query_pool(deps: &DepsMut<ElysQuery>, pool_id: u64) -> StdResult<PoolResponse> {
     let query = ElysQuery::QueryGetPool { pool_id };
-    let query_pool_response: Option<QueryGetPoolResponse> = deps.querier.query(&query.into()).ok();
+    let query_pool_response: QueryGetPoolResponse = deps.querier.query(&query.into())
+        .map_err(|e| StdError::generic_err(format!("Failed to query pool {}: {}", pool_id, e)))?;
 
-    // Check if pool exists, otherwise return an error
-    let pool_response =
-        query_pool_response.ok_or_else(|| StdError::generic_err("failed to query pool"))?;
-    let pool = pool_response
+    let pool = query_pool_response
         .pool
-        .ok_or_else(|| StdError::generic_err("pool not found"))?;
+        .ok_or_else(|| StdError::generic_err(format!("Pool {} not found", pool_id)))?;
 
     Ok(pool)
 }
📝 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.

Suggested change
pub fn query_pool(deps: &DepsMut<ElysQuery>, pool_id: u64) -> StdResult<PoolResponse> {
let query = ElysQuery::QueryGetPool { pool_id };
let query_pool_response: Option<QueryGetPoolResponse> = deps.querier.query(&query.into()).ok();
// Check if pool exists, otherwise return an error
let pool_response =
query_pool_response.ok_or_else(|| StdError::generic_err("failed to query pool"))?;
let pool = pool_response
.pool
.ok_or_else(|| StdError::generic_err("pool not found"))?;
Ok(pool)
}
pub fn query_pool(deps: &DepsMut<ElysQuery>, pool_id: u64) -> StdResult<PoolResponse> {
let query = ElysQuery::QueryGetPool { pool_id };
let query_pool_response: QueryGetPoolResponse = deps.querier.query(&query.into())
.map_err(|e| StdError::generic_err(format!("Failed to query pool {}: {}", pool_id, e)))?;
let pool = query_pool_response
.pool
.ok_or_else(|| StdError::generic_err(format!("Pool {} not found", pool_id)))?;
Ok(pool)
}
🤖 Prompt for AI Agents
In packages/lending-utils/src/elys.rs around lines 111 to 123, the error
handling in query_pool currently replaces original errors with generic messages,
losing valuable debugging information. Modify the code to capture the original
error from the query call and propagate it or include its details in the
returned StdError. This can be done by using map_err or a similar method to
convert the original error into a StdError while preserving the original error
message instead of using generic_err with a fixed string.

🛠️ Refactor suggestion

Improve error handling and function signature.

The function has several issues that should be addressed:

  1. Uses &DepsMut but only reads data - should use &Deps
  2. Swallows actual error information with .ok()
  3. Generic error messages aren't helpful for debugging
-pub fn query_pool(deps: &DepsMut<ElysQuery>, pool_id: u64) -> StdResult<PoolResponse> {
+/// Queries pool information by pool ID from the Elys network
+pub fn query_pool(deps: &Deps<ElysQuery>, pool_id: u64) -> StdResult<PoolResponse> {
     let query = ElysQuery::QueryGetPool { pool_id };
-    let query_pool_response: Option<QueryGetPoolResponse> = deps.querier.query(&query.into()).ok();
+    let query_pool_response: QueryGetPoolResponse = deps.querier.query(&query.into())
+        .map_err(|e| StdError::generic_err(format!("Failed to query pool {}: {}", pool_id, e)))?;

-    // Check if pool exists, otherwise return an error
-    let pool_response =
-        query_pool_response.ok_or_else(|| StdError::generic_err("failed to query pool"))?;
-    let pool = pool_response
+    let pool = query_pool_response
         .pool
-        .ok_or_else(|| StdError::generic_err("pool not found"))?;
+        .ok_or_else(|| StdError::generic_err(format!("Pool {} not found", pool_id)))?;

     Ok(pool)
 }
📝 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.

Suggested change
pub fn query_pool(deps: &DepsMut<ElysQuery>, pool_id: u64) -> StdResult<PoolResponse> {
let query = ElysQuery::QueryGetPool { pool_id };
let query_pool_response: Option<QueryGetPoolResponse> = deps.querier.query(&query.into()).ok();
// Check if pool exists, otherwise return an error
let pool_response =
query_pool_response.ok_or_else(|| StdError::generic_err("failed to query pool"))?;
let pool = pool_response
.pool
.ok_or_else(|| StdError::generic_err("pool not found"))?;
Ok(pool)
}
/// Queries pool information by pool ID from the Elys network
pub fn query_pool(deps: &Deps<ElysQuery>, pool_id: u64) -> StdResult<PoolResponse> {
let query = ElysQuery::QueryGetPool { pool_id };
let query_pool_response: QueryGetPoolResponse = deps
.querier
.query(&query.into())
.map_err(|e| StdError::generic_err(format!("Failed to query pool {}: {}", pool_id, e)))?;
let pool = query_pool_response
.pool
.ok_or_else(|| StdError::generic_err(format!("Pool {} not found", pool_id)))?;
Ok(pool)
}
🤖 Prompt for AI Agents
In packages/lending-utils/src/elys.rs around lines 111 to 123, the function
query_pool uses &DepsMut when it only reads data, swallows error details by
using .ok(), and returns generic error messages. Change the function signature
to accept &Deps instead of &DepsMut, replace .ok() with proper error propagation
using the ? operator to preserve error details, and update error messages to
include specific context from the original errors to improve debugging.

Comment on lines +27 to +70
"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
}
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Placeholder keys in LibraryAccountType need replacement.
The LibraryAccountType definition is using placeholder property names like |library_account_addr|, |account_id|, and |library_id|. These must be replaced with the actual field names (e.g., "library_account_addr", "account_id", "library_id") for the schema to validate correctly.

🤖 Prompt for AI Agents
In contracts/libraries/elys-lending/schema/valence-elys-lending.json between
lines 27 and 70, replace the placeholder keys surrounded by pipes such as
"|library_account_addr|", "|account_id|", and "|library_id|" with the actual
field names without pipes, like "library_account_addr", "account_id", and
"library_id". This change is necessary to ensure the JSON schema validates
correctly.

Comment on lines +342 to +386
"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
}
]
},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Critical: Placeholder keys repeated in second LibraryAccountType.
The duplicate LibraryAccountType under FunctionMsgs also contains |library_account_addr|, |account_id|, and |library_id| placeholders. Please replace these with actual field names or adjust generation tooling.

🤖 Prompt for AI Agents
In contracts/libraries/elys-lending/schema/valence-elys-lending.json between
lines 342 and 386, the placeholder keys like "|library_account_addr|",
"|account_id|", and "|library_id|" are repeated in the second LibraryAccountType
under FunctionMsgs. Replace these placeholder keys with actual, meaningful field
names that reflect their purpose or adjust the schema generation tooling to
output correct field names instead of placeholders.

@stana-miric stana-miric force-pushed the smiric/elys-lending branch 2 times, most recently from 20a96d9 to 5867c48 Compare June 2, 2025 12:56
@stana-miric stana-miric force-pushed the smiric/elys-lending branch from 5867c48 to a6a2468 Compare June 2, 2025 12:59
@stana-miric stana-miric marked this pull request as draft June 4, 2025 06:34
@stana-miric stana-miric marked this pull request as ready for review June 5, 2025 06:06
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Nitpick comments (5)
docs/src/libraries/cosmwasm/elys_lending.md (5)

1-1: Title-case the main heading.

The top-level heading should use title case for consistency.
Apply this diff:

-# Valence Elys Lending library
+# Valence Elys Lending Library

5-5: Hyphenate the compound adjective.

“High Level Flow” should be “High-Level Flow” to correctly form the compound adjective.

-## High Level Flow
+## High-Level Flow
🧰 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: Elys Lendin...

(EN_COMPOUND_ADJECTIVE_INTERNAL)


19-23: Unify the naming of the claim operation.

In the mermaid diagram you use “Claim” in step 1 and “Claim Rewards” in step 5, but the function name is ClaimRewards. Consider renaming both to ClaimRewards for consistency with the rest of the docs and codebase.


39-48: Show derive macros in the config snippet.

The actual LibraryConfig struct in Rust likely derives serialization, schema, and debug traits. Reflect that in the snippet for accuracy:

 ```rust
+#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, JsonSchema)]
 pub struct LibraryConfig {
     /// Address of the input account
     pub input_addr: LibraryAccountType,
     /// Address of the output account
     pub output_addr: LibraryAccountType,
     /// ID of the pool we are going to lend to
     pub pool_id: Uint64,
 }

---

`37-38`: **Link key types to their definitions.**

To improve discoverability, link `LibraryConfig`, `LibraryAccountType`, and `Uint64` to their code definitions or API docs. E.g.:

> The library is configured on instantiation via the [`LibraryConfig`](#LibraryConfig) type.

</blockquote></details>

</blockquote></details>

<details>
<summary>📜 Review details</summary>

**Configuration used: CodeRabbit UI**
**Review profile: CHILL**
**Plan: Pro**


<details>
<summary>📥 Commits</summary>

Reviewing files that changed from the base of the PR and between a6a24682f8fe9c2449a85c2e52ea1e1954822e6e and 077bd9d7d361eaeee4156d13046b7e0684e5ee70.

</details>

<details>
<summary>📒 Files selected for processing (2)</summary>

* `docs/src/SUMMARY.md` (1 hunks)
* `docs/src/libraries/cosmwasm/elys_lending.md` (1 hunks)

</details>

<details>
<summary>✅ Files skipped from review due to trivial changes (1)</summary>

* docs/src/SUMMARY.md

</details>

<details>
<summary>🧰 Additional context used</summary>

<details>
<summary>🪛 LanguageTool</summary>

<details>
<summary>docs/src/libraries/cosmwasm/elys_lending.md</summary>

[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: Elys Lendin...

(EN_COMPOUND_ADJECTIVE_INTERNAL)

---

[grammar] ~60-~60: Using ‘plenty’ without ‘of’ is considered to be informal.
Context: ...n the vault for the input account. 2. **Amount Calculation**: Uses the exact amount if specified; ...

(PLENTY_OF_NOUNS)

</details>

</details>

</details>

</details>

<!-- This is an auto-generated comment by CodeRabbit for review status -->

Copy link
Contributor

@keyleu keyleu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's wait to merge until tested on Elys, hopefully soon.

Only have a question regarding rewards as normally for most protocols the rewards are automatically claimed when unbonding, is this not the case for Elys?

Comment on lines +9 to +17
#[cw_serde]
pub enum FunctionMsgs {
/// Message to lend tokens.
Lend {},
/// Message to withdraw tokens. If amount is not specified, full amount will be withdrawn.
Withdraw { amount: Option<Uint128> },
/// Message to claim reward tokens.
ClaimRewards {},
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are rewards not automatically claimed when unbonding?
Depending on the case I see a possible improvement that can be done here:

  • If not automatically claimed, when we withdraw, we also claim rewards and send them to the output account.
  • If automatically claimed, when we withdraw we need to send the rewards claimed as well otherwise they will be stuck in the input account

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unfortunately, the rewards aren’t claimed automatically, although they mentioned in the Telegram group that they would fix it so they get claimed by default. I could ask if that has been resolved.
Either way, I agree that it would be nicer if claiming happened automatically when withdrawing. I’m just not sure whether it’s still better to keep the claim as a separate function or remove it entirely?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, I can see maybe we want to keep it in case we want to claim rewards and keep lending? I'm fine with keeping it since you already wrote it.

Yeah if they are going to change claim rewards we probably have to wait until they change it and then adjust the library to send the claimed rewards on withdraw.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants