fix(ui): improve platform address validation consistency across UI screens#592
Conversation
The hint text showed "y..." which is a Core address prefix. Platform addresses use evo1.../tevo1... (Bech32m format per DIP-18). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n withdraw screen The withdraw screen only supports Core addresses but had no hint text and showed a generic "Invalid address" for platform addresses. Now shows network-appropriate Core address hints and a specific error message when a user enters an evo1.../tevo1... platform address. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The send dialog hint text showed "y..." which is only valid for testnet/devnet. Now shows network-appropriate Core address prefixes (X.../7... for mainnet, y.../8... for testnet). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The send dialog had no frontend address validation — invalid addresses only errored at the backend level. Now validates on input change using Address parsing with inline error display, and detects platform addresses (evo1.../tevo1...) with a specific unsupported format message. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
| ui.add( | ||
| egui::TextEdit::singleline(&mut self.platform_address_input) | ||
| .hint_text("Enter Platform address (y...)") | ||
| .hint_text("Enter Platform address (evo1.../tevo1...)") |
There was a problem hiding this comment.
we use dash/tdash right? not evo/tevo
There was a problem hiding this comment.
You're right — fixing all evo/tevo references to dash/tdash per DIP-18. Also adding a shared is_platform_address() helper that uses the SDK's PLATFORM_HRP_MAINNET/PLATFORM_HRP_TESTNET constants + the 1 separator instead of hardcoding prefixes. Force push incoming.
There was a problem hiding this comment.
You're right — the SDK uses dash/tdash as the HRP (from DIP-18), so addresses are dash1.../tdash1.... I copied the stale evo1/tevo1 from the existing codebase. Fixing now.
📝 WalkthroughWalkthroughUI and docs updated to adopt DIP-18 naming and detection for Platform Bech32m addresses (dash1/tdash1). Added a lightweight is_platform_address_string helper, made address input hints network-aware, trim-and-validate inputs, reject platform Bech32m where Core (base58) addresses are required, and surface clearer error text. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/ui/identities/transfer_screen.rs (1)
248-253: Consider making the platform address hint network-aware for consistency.The other screens in this PR (
withdraw_screen.rs,dialogs.rs) branch onself.app_context.network == Network::Dashto show network-specific hints. This screen shows bothevo1.../tevo1...regardless of network, which is slightly inconsistent and can confuse users (e.g., showingtevo1...on mainnet).Suggested network-aware hint
- .hint_text("Enter Platform address (evo1.../tevo1...)") + .hint_text(if self.app_context.network == dash_sdk::dashcore_rpc::dashcore::Network::Dash { + "Enter Platform address (evo1...)" + } else { + "Enter Platform address (tevo1...)" + })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/identities/transfer_screen.rs` around lines 248 - 253, Update the platform address hint to be network-aware: in the UI code that builds the TextEdit for self.platform_address_input (the block where egui::TextEdit::singleline is used), check self.app_context.network (the same enum used elsewhere like Network::Dash in withdraw_screen.rs and dialogs.rs) and set the hint_text accordingly (e.g., "Enter Platform address (evo1...)" for Dash vs "Enter Platform address (tevo1...)" for other networks). Modify only the hint text construction so it mirrors the branching pattern used in withdraw_screen.rs/dialogs.rs, referencing self.app_context.network and Network::Dash to choose the correct hint.src/ui/wallets/wallets_screen/dialogs.rs (1)
222-233: Send button is not disabled whenaddress_erroris present.The withdraw screen (line 576) gates its button with
!has_address_error, but here the Send button remains clickable even when the inline validation shows an error. Consider disabling the button for consistency:Suggested fix
+ let has_address_error = self.send_dialog.address_error.is_some(); ui.horizontal(|ui| { - if ui.button("Send").clicked() { + if ui.add_enabled(!has_address_error, egui::Button::new("Send")).clicked() { match self.prepare_send_action() {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/wallets/wallets_screen/dialogs.rs` around lines 222 - 233, The Send button is still clickable even when there's an inline address validation error; update the UI to disable the Send button when send_dialog has an address_error (mirror the withdraw screen's gating). Wrap the button in an enabled/disabled container (e.g., use ui.add_enabled(...) or equivalent) checking !self.send_dialog.address_error.is_some() (or a has_address_error helper) so the existing click/prepare_send_action() path only runs when no address error; refer to send_dialog, prepare_send_action, and the Send button block to apply the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/ui/identities/withdraw_screen.rs`:
- Around line 160-181: The validation currently trims input into the local
variable trimmed but still calls Address::from_str(&self.withdrawal_address),
causing valid addresses with surrounding whitespace to fail; update the
validation in the response.changed() branch to call Address::from_str(trimmed)
(and on success clear self.withdrawal_address_error, on error set it to "Invalid
Core address") and ensure any other checks (e.g., starts_with("evo1") / "tevo1")
continue to use trimmed so all validation uses the trimmed string.
---
Nitpick comments:
In `@src/ui/identities/transfer_screen.rs`:
- Around line 248-253: Update the platform address hint to be network-aware: in
the UI code that builds the TextEdit for self.platform_address_input (the block
where egui::TextEdit::singleline is used), check self.app_context.network (the
same enum used elsewhere like Network::Dash in withdraw_screen.rs and
dialogs.rs) and set the hint_text accordingly (e.g., "Enter Platform address
(evo1...)" for Dash vs "Enter Platform address (tevo1...)" for other networks).
Modify only the hint text construction so it mirrors the branching pattern used
in withdraw_screen.rs/dialogs.rs, referencing self.app_context.network and
Network::Dash to choose the correct hint.
In `@src/ui/wallets/wallets_screen/dialogs.rs`:
- Around line 222-233: The Send button is still clickable even when there's an
inline address validation error; update the UI to disable the Send button when
send_dialog has an address_error (mirror the withdraw screen's gating). Wrap the
button in an enabled/disabled container (e.g., use ui.add_enabled(...) or
equivalent) checking !self.send_dialog.address_error.is_some() (or a
has_address_error helper) so the existing click/prepare_send_action() path only
runs when no address error; refer to send_dialog, prepare_send_action, and the
Send button block to apply the change.
…tevo prefixes
- Add is_platform_address_string() helper using SDK's PLATFORM_HRP_MAINNET/TESTNET constants
- Replace all hardcoded starts_with("evo1")/starts_with("tevo1") checks with the helper
- Update all UI hint text, error messages, and doc comments to reference dash1.../tdash1...
- The '1' is the bech32 separator, not part of the HRP — detection now properly uses HRP + separator
Addresses review feedback on PR dashpay#592.
80e8d1e to
f9d1859
Compare
- Use trimmed input for Address::from_str in withdraw_screen (bug fix) - Make transfer_screen hint text network-aware (dash1 vs tdash1) - Disable Send button when address validation error is present
thephez
left a comment
There was a problem hiding this comment.
Partially tested ACK (identity create worked)
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (2)
src/ui/helpers.rs (2)
1-17: Run clippy and fmt before finalizing.Please run
cargo clippyandcargo +nightly fmtto conform with the repo’s Rust workflow checks.As per coding guidelines:
**/*.rs: Always run cargo clippy and cargo +nightly fmt when finalizing work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/helpers.rs` around lines 1 - 17, Run the repository Rust linters and formatter on your changes: execute cargo clippy and cargo +nightly fmt, then fix any warnings/errors reported (especially in the is_platform_address_string function and any use of PLATFORM_HRP_MAINNET / PLATFORM_HRP_TESTNET) so code conforms to project style and clippy suggestions before finalizing the PR.
4-17: Add a tiny inline test for the new helper.This is small but easy to lock down with a couple of prefix/negative cases.
💡 Suggested inline test
+#[cfg(test)] +mod tests { + use super::is_platform_address_string; + + #[test] + fn detects_platform_hrp_prefixes() { + assert!(is_platform_address_string("dash1qqqq")); + assert!(is_platform_address_string("tdash1qqqq")); + assert!(!is_platform_address_string("evo1qqqq")); + assert!(!is_platform_address_string("XyZ123")); + } +}As per coding guidelines:
src/**/*.rs: Unit tests should be inline in source files using #[test] attribute; UI integration tests in tests/kittest/; E2E tests in tests/e2e/.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/helpers.rs` around lines 4 - 17, Add an inline unit test for is_platform_address_string by creating a #[cfg(test)] mod tests in the same file and a #[test] fn (e.g., test_is_platform_address_string) that asserts true for strings built from PLATFORM_HRP_MAINNET and PLATFORM_HRP_TESTNET followed by '1' and some payload (e.g., format!("{}1{}", PLATFORM_HRP_MAINNET, "abcd")), and asserts false for negative cases like wrong HRP (e.g., "x1abcd"), missing separator (e.g., format!("{}abcd", PLATFORM_HRP_MAINNET)), and other invalid prefixes; reference the function is_platform_address_string and the constants PLATFORM_HRP_MAINNET and PLATFORM_HRP_TESTNET in the test to lock behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/ui/helpers.rs`:
- Around line 1-17: Run the repository Rust linters and formatter on your
changes: execute cargo clippy and cargo +nightly fmt, then fix any
warnings/errors reported (especially in the is_platform_address_string function
and any use of PLATFORM_HRP_MAINNET / PLATFORM_HRP_TESTNET) so code conforms to
project style and clippy suggestions before finalizing the PR.
- Around line 4-17: Add an inline unit test for is_platform_address_string by
creating a #[cfg(test)] mod tests in the same file and a #[test] fn (e.g.,
test_is_platform_address_string) that asserts true for strings built from
PLATFORM_HRP_MAINNET and PLATFORM_HRP_TESTNET followed by '1' and some payload
(e.g., format!("{}1{}", PLATFORM_HRP_MAINNET, "abcd")), and asserts false for
negative cases like wrong HRP (e.g., "x1abcd"), missing separator (e.g.,
format!("{}abcd", PLATFORM_HRP_MAINNET)), and other invalid prefixes; reference
the function is_platform_address_string and the constants PLATFORM_HRP_MAINNET
and PLATFORM_HRP_TESTNET in the test to lock behavior.
Summary
After PR #575 introduced DIP-18 HRP support (
evo1.../tevo1...Bech32m platform addresses), several UI screens had stale hint text or inconsistent address validation. This PR fixes 4 issues to make the UI consistent with the backend.Reported by @lklimek — the platform address validation in input fields had improper validation where the backend checked properly but the frontend did not.
Changes (4 commits)
1.
fix(ui): correct platform address hint text in transfer screenFile:
src/ui/identities/transfer_screen.rs"Enter Platform address (y...)"to"Enter Platform address (evo1.../tevo1...)"validate_platform_address()already handled both formats correctly; only the hint was stale2.
fix(ui): add network-aware hint text and platform address detection in withdraw screenFile:
src/ui/identities/withdraw_screen.rs"Enter Core address (X.../7...) or Platform address (evo1.../tevo1...)"evo1/tevo1) to auto-switch theis_platform_transfertoggley-prefix addresses as platform transfers3.
fix(ui): use network-aware hint text in wallet send dialogFile:
src/ui/wallet/send_screen.rs"Enter Core (X.../7...) or Platform (evo1...) address""Enter Core (y.../8...) or Platform (tevo1...) address"4.
fix(ui): add inline address validation to wallet send dialogFile:
src/ui/wallet/send_screen.rsAddress::from_str()andPlatformAddress::from_bech32m_string()for comprehensive validationTesting
cargo checkBreaking Changes
None.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation