fix(fiat): add MWK emoji and fix load_fiat_values compilation #153
Conversation
Added mw emoji for the Malawi Kwacha (MWK) Changed return type to Result<FiatList> to allow use of allow of ? operator Fixed missing closing brace in for loop Removed temporary backup file src/fiat.rs~ and updated .gitignore
WalkthroughThe PR simplifies build configuration by updating .gitignore patterns, refactors MOSTRO_PUBKEY initialization in the CLI module to resolve from CLI arguments or environment variables with centralized error handling, and introduces Result-based error propagation to the fiat values loader function. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/fiat.rs (1)
40-60: Update caller to handle Result return type.The function
check_currency_tickercallsload_fiat_values()on line 45, but doesn't handle theResultreturn type after the signature change. This will cause a compilation error.🔎 Proposed fix
pub fn check_currency_ticker(currency: String) -> Option<String> { let upper = currency.to_uppercase(); let mut selectedcurrency: Option<String> = None; let mut description = String::new(); - let list = load_fiat_values(); + let list = load_fiat_values().ok()?; for curr in list.iter() { if curr.0 == upper { selectedcurrency = Some(curr.0.to_owned()); description = curr.1.to_owned(); } } match selectedcurrency.clone() { Some(s) => println!("You have selected all offers of {} - {}", s, description), None => println!("Mmmmhhh you shouldn't be arrived here...something bad!"), } selectedcurrency }Alternatively, propagate the error by changing the function signature to return
Result<Option<String>>.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.gitignoresrc/.fiat.rs.un~src/cli.rssrc/fiat.rs
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 135
File: src/cli.rs:301-306
Timestamp: 2025-09-12T20:02:14.291Z
Learning: The mostro-cli is designed as a test client that allows users to test both regular user operations and admin operations from the terminal. Therefore, MOSTRO_PUBKEY should always be required regardless of whether NSEC_PRIVKEY is present, as both user and admin operations need to know which Mostro instance to interact with.
📚 Learning: 2025-09-12T20:02:14.291Z
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 135
File: src/cli.rs:301-306
Timestamp: 2025-09-12T20:02:14.291Z
Learning: The mostro-cli is designed as a test client that allows users to test both regular user operations and admin operations from the terminal. Therefore, MOSTRO_PUBKEY should always be required regardless of whether NSEC_PRIVKEY is present, as both user and admin operations need to know which Mostro instance to interact with.
Applied to files:
src/cli.rs
📚 Learning: 2025-09-09T19:58:58.506Z
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 135
File: src/cli/add_invoice.rs:80-82
Timestamp: 2025-09-09T19:58:58.506Z
Learning: In the mostro-cli codebase, when the trade_index parameter isn't needed inside a function like wait_for_dm, it should be made Optional<i64> instead of requiring callers to pass hardcoded values like 0. This makes the API clearer and avoids unnecessary computation for resolving indices when they're not used.
Applied to files:
src/cli.rs
📚 Learning: 2025-09-14T14:01:01.728Z
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 135
File: src/util.rs:614-619
Timestamp: 2025-09-14T14:01:01.728Z
Learning: In the mostro-cli codebase, the user arkanoider indicated that accessing `public_key` without parentheses (as a field) is valid, even though most of the codebase uses `public_key()` method calls. This suggests there may be `Deref` implementation or other Rust features that allow both syntaxes to work.
Applied to files:
src/cli.rs
📚 Learning: 2025-09-13T10:31:42.313Z
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 135
File: src/cli/get_dm.rs:24-29
Timestamp: 2025-09-13T10:31:42.313Z
Learning: In mostro-cli's get_dm.rs, the message filtering is intentional: non-admin users should only receive DirectMessage from users, while messages from Mostro are only fetched when the admin flag is true. Don't include GiftWrap messages for non-admin users.
Applied to files:
src/cli.rs
📚 Learning: 2025-09-13T20:48:31.581Z
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 135
File: src/cli/send_msg.rs:89-110
Timestamp: 2025-09-13T20:48:31.581Z
Learning: In mostro-cli's send_msg.rs, tokio::spawn with async move is valid for sending DMs when PublicKey implements Copy, and arkanoider confirmed this approach works without memory issues or compilation errors.
Applied to files:
src/cli.rs
📚 Learning: 2025-09-13T20:48:31.581Z
Learnt from: arkanoider
Repo: MostroP2P/mostro-cli PR: 135
File: src/cli/send_msg.rs:89-110
Timestamp: 2025-09-13T20:48:31.581Z
Learning: In mostro-cli's send_msg.rs, arkanoider confirmed that tokio::spawn with async move is correct and compiles without errors when using PublicKey (which implements Copy). The spawning approach is acceptable for DM sending in this context.
Applied to files:
src/cli.rs
🔇 Additional comments (4)
.gitignore (1)
1-1: LGTM!Standard pattern to ignore editor backup files. Clean addition.
src/cli.rs (2)
373-385: LGTM! Centralizes MOSTRO_PUBKEY resolution with proper error handling.The resolver correctly prioritizes CLI arguments over environment variables and provides clear error messages. This aligns with the requirement that MOSTRO_PUBKEY should always be set for all operations. Based on learnings, this is correctly enforced for both user and admin operations.
420-421: LGTM! Proper usage of the resolver function.The implementation correctly uses the resolver with error propagation and parses the result into PublicKey.
src/fiat.rs (1)
1467-1467: LGTM! Proper error handling for JSON parsing.The error handling correctly uses
map_errto wrap parsing errors with context.
| } | ||
|
|
||
| pub fn load_fiat_values() -> FiatList { | ||
| pub fn load_fiat_values() -> Result<FiatList> { |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -n 70 src/fiat.rs | cat -nRepository: MostroP2P/mostro-cli
Length of output: 2269
🏁 Script executed:
rg -n "use.*Result|use.*anyhow" src/fiat.rsRepository: MostroP2P/mostro-cli
Length of output: 46
🏁 Script executed:
rg "use.*anyhow|use.*Result" --type rust -m 5Repository: MostroP2P/mostro-cli
Length of output: 1516
🏁 Script executed:
sed -n '62,120p' src/fiat.rs | cat -nRepository: MostroP2P/mostro-cli
Length of output: 1979
🏁 Script executed:
rg -A 200 "pub fn load_fiat_values" src/fiat.rs | tail -n 20Repository: MostroP2P/mostro-cli
Length of output: 561
🏁 Script executed:
rg -A 500 "pub fn load_fiat_values" src/fiat.rs | rg -B 2 "^}$|Ok\(|return"Repository: MostroP2P/mostro-cli
Length of output: 46
🏁 Script executed:
wc -l src/fiat.rsRepository: MostroP2P/mostro-cli
Length of output: 80
🏁 Script executed:
awk '/pub fn load_fiat_values/,/^}$/' src/fiat.rs | tail -n 30Repository: MostroP2P/mostro-cli
Length of output: 818
🏁 Script executed:
sed -n '40,60p' src/fiat.rs | cat -nRepository: MostroP2P/mostro-cli
Length of output: 865
Add missing Result import and fix multiple compilation errors.
The function declares return type Result<FiatList> but is missing use anyhow::Result; import. Additionally:
- Line 1470: Function returns
ok(fiatlist)instead ofOk(fiatlist)(lowercase should be uppercase) - Line 45-47: Calling code in
check_currency_ticker()doesn't handle the Result type—it assigns the result directly tolistand calls.iter()without unwrapping. This needs to handle theResultwith either?operator,.unwrap(), or.unwrap_or_else()
🤖 Prompt for AI Agents
In src/fiat.rs around line 62 (and related spots at lines ~45-47 and ~1470), add
the missing import `use anyhow::Result;`, change the lowercase `ok(fiatlist)`
return at ~1470 to `Ok(fiatlist)`, and update the call site in
check_currency_ticker() (lines 45-47) to handle the Result returned by
load_fiat_values() — either propagate the error with the `?` operator,
explicitly unwrap with `.unwrap()` or supply a fallback via `.unwrap_or_else()`
depending on desired error handling; ensure the variable `list` is assigned the
unwrapped FiatList before calling `.iter()`.
| "MWK": { | ||
| "symbol": "MWK", | ||
| "name": "Malawian Kwacha", | ||
| "symbol_native": "MK", | ||
| "decimal_digits": 2, | ||
| "rounding": 0, | ||
| "code": "MWK, | ||
| "emoji": "mw", | ||
| "name_plural": "Malawian kwachas" | ||
| }, |
There was a problem hiding this comment.
Fix JSON syntax error and emoji value.
The MWK entry has critical issues:
- Line 912: Missing closing quote in
"code": "MWK,- should be"code": "MWK", - Line 913: Emoji is incorrectly set to
"mw"instead of the Malawi flag emoji"🇲🇼"
These will cause JSON parsing to fail at runtime.
🔎 Proposed fix
"MWK": {
"symbol": "MWK",
"name": "Malawian Kwacha",
"symbol_native": "MK",
"decimal_digits": 2,
"rounding": 0,
- "code": "MWK,
- "emoji": "mw",
+ "code": "MWK",
+ "emoji": "🇲🇼",
"name_plural": "Malawian kwachas"
},📝 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.
| "MWK": { | |
| "symbol": "MWK", | |
| "name": "Malawian Kwacha", | |
| "symbol_native": "MK", | |
| "decimal_digits": 2, | |
| "rounding": 0, | |
| "code": "MWK, | |
| "emoji": "mw", | |
| "name_plural": "Malawian kwachas" | |
| }, | |
| "MWK": { | |
| "symbol": "MWK", | |
| "name": "Malawian Kwacha", | |
| "symbol_native": "MK", | |
| "decimal_digits": 2, | |
| "rounding": 0, | |
| "code": "MWK", | |
| "emoji": "🇲🇼", | |
| "name_plural": "Malawian kwachas" | |
| }, |
🤖 Prompt for AI Agents
In src/fiat.rs around lines 906 to 915, the MWK JSON entry has a missing closing
quote on the "code" field and the emoji value is incorrect; fix by changing
`"code": "MWK,` to `"code": "MWK",` and set `"emoji": "mw",` to the correct
Malawi flag emoji `"emoji": "🇲🇼",` ensuring proper UTF-8 encoding and valid
JSON quoting.
| fiatlist.sort_by(|a, b| a.0.cmp(&b.0)); | ||
|
|
||
| fiatlist | ||
| ok(fiatlist) |
There was a problem hiding this comment.
Fix return statement - use Ok instead of ok.
The return statement uses lowercase ok(fiatlist) which will cause a compilation error. Rust's Result enum requires uppercase Ok.
🔎 Proposed fix
- ok(fiatlist)
+ Ok(fiatlist)📝 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.
| ok(fiatlist) | |
| Ok(fiatlist) |
🤖 Prompt for AI Agents
In src/fiat.rs around line 1478, the function returns `ok(fiatlist)` which is
invalid because Rust's Result uses the capitalized `Ok`; change the return to
`Ok(fiatlist)` so the function returns the correct Result variant (no other
changes needed unless type mismatches surface).
|
Hi @codaMW, why to do these changes on |
|
Yes please @codaMW check lib.rs file, you won't see fiat module exposed there... |
|
Hi maintainers
Thanks for the patience I accidentally let this PR grow beyond its
original scope while fixing editor artifacts.
I’ve now reset it to *only include the MWK currency addition*, removed
temporary/backup files, added ignore rules, and verified the build locally.
Appreciate the guidance and further clarifications
…On Wed, Jan 7, 2026 at 12:09 PM arkanoider ***@***.***> wrote:
*arkanoider* left a comment (MostroP2P/mostro-cli#153)
<#153 (comment)>
Yes please @codaMW <https://github.com/codaMW> check lib.rs file, you
won't see fiat module exposed there...
—
Reply to this email directly, view it on GitHub
<#153 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BBCHOJKNVB2GTOARRV2ZSQD4FTLO7AVCNFSM6AAAAACQU2WALCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMJYGE3DIMZWHE>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Hi @codaMW I'm closing this PR as the requests are not being addressed, there are several errors included on this PR, please read all comments and if you want you can create a new clean PR |
|
Hi @grunch, thanks for the feedback and for taking the time to review the
PR. I understand the concerns and I apologize for not addressing all the
comments properly. I’ll go through the feedback carefully, fix the issues,
and open a new clean PR once everything is corrected. Appreciate the
guidance
…On Wed, Jan 7, 2026 at 9:04 PM Francisco Calderón ***@***.***> wrote:
*grunch* left a comment (MostroP2P/mostro-cli#153)
<#153 (comment)>
Hi @codaMW <https://github.com/codaMW> I'm closing this PR as the
requests are not being addressed, there are several errors included on this
PR, please read all comments and if you want you can create a new clean PR
—
Reply to this email directly, view it on GitHub
<#153 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BBCHOJMYIJ5JUSVVESQRM334FVKE3AVCNFSM6AAAAACQU2WALCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZTOMRQGMZTOMBTGU>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
3 participants
This PR addresses several issues in
fiat.rs:load_fiat_valuesreturn type toResult<FiatList>for proper error handlingfiat.rs~*~to .gitignore to prevent committing future backup filesSummary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.