Skip to content

fix(v2): remove manual walletname encoding and rely on generated client path serialization #1209

Merged
theborakompanioni merged 4 commits intojoinmarket-webui:v2from
Ashish-Kumar-Dash:fix/v2-walletname-encoding-cleanup
Apr 16, 2026
Merged

fix(v2): remove manual walletname encoding and rely on generated client path serialization #1209
theborakompanioni merged 4 commits intojoinmarket-webui:v2from
Ashish-Kumar-Dash:fix/v2-walletname-encoding-cleanup

Conversation

@Ashish-Kumar-Dash
Copy link
Copy Markdown
Contributor

@Ashish-Kumar-Dash Ashish-Kumar-Dash commented Apr 15, 2026

Summary

This PR removes manual walletname URL encoding at generated-client callsites and standardizes on passing raw walletname values.

Why

The generated API client already encodes path params internally. Pre-encoding at callsites can cause double-encoding edge cases, especially for wallets created outside Jam.

What changed

  • Replaced manual walletname encoding with raw walletname in generated-client path usage.
  • Kept behavior consistent with the existing raw walletname flow.

Validation

  • Lint: passes (only existing warnings, no new ones).
  • Build: passes.
  • Targeted unit tests for touched send/sweep logic: pass.

Notes

No functional API shape changes were introduced; this is a consistency and safety cleanup.

@Ashish-Kumar-Dash Ashish-Kumar-Dash force-pushed the fix/v2-walletname-encoding-cleanup branch from 47b502a to 67063b8 Compare April 15, 2026 16:57
@Ashish-Kumar-Dash Ashish-Kumar-Dash changed the title Fix/v2 walletname encoding cleanup fix(v2): remove manual walletname encoding and rely on generated client path serialization Apr 15, 2026
@theborakompanioni theborakompanioni added bug Something isn't working rpc-api A feature or functionality that aims to modify Joinmarket api's labels Apr 15, 2026
@parrth20
Copy link
Copy Markdown
Member

@Ashish-Kumar-Dash LGTM thanks!
but i think we could we add a small regression test for a wallet name containing a space or % to lock this behavior down? since this fix relies on the generated path serializer encoding the wallet name exactly once, a test would help make sure we don’t accidentally reintroduce manual pre-encoding later.
what are your thoughts ???

@Ashish-Kumar-Dash
Copy link
Copy Markdown
Contributor Author

Hey! I agree & have added a small regression unit test, do check it out

@parrth20 parrth20 self-requested a review April 16, 2026 12:15
Copy link
Copy Markdown
Member

@parrth20 parrth20 left a comment

Choose a reason for hiding this comment

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

lint errors !!

@Ashish-Kumar-Dash
Copy link
Copy Markdown
Contributor Author

yup yup I was fixing them only, didn't expect for a review to be this fast :)

@theborakompanioni theborakompanioni merged commit e2519d4 into joinmarket-webui:v2 Apr 16, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working rpc-api A feature or functionality that aims to modify Joinmarket api's

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants