Skip to content

fix/957 sign message button long message - #967

Closed
JakeUrban wants to merge 4 commits into
mainfrom
fix/957-sign-message-button-long-message
Closed

fix/957 sign message button long message#967
JakeUrban wants to merge 4 commits into
mainfrom
fix/957-sign-message-button-long-message

Conversation

@JakeUrban

Copy link
Copy Markdown
Contributor
  • fix(walletkit): keep sign button visible for long dApp messages
  • fix(walletkit): raise sign_message size cap from 1KB to 10KB
  • fix(walletkit): let the message box shrink when security warnings need room

SmolinPavel and others added 4 commits July 30, 2026 21:27
The SEP-53 message ScrollView had no height bound, so a long message
grew the bottom sheet content past the screen — clipping the
Cancel/Confirm buttons — and the unbounded ScrollView never scrolled,
making the tail of the message unreachable.

Cap the message area at 30% of screen height (mirroring the auth-entry
sheet's capped XDR view) so long messages scroll inside the box and the
action buttons always stay on screen.

Fixes #957

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The 1KB cap was attributed to SEP-53, but the spec imposes no message
size limit and the browser extension enforces none. dApps signing JSON
payloads routinely exceed 1KB and were rejected with "Message too long"
before the sign sheet even opened.

Keep a sanity cap against absurd WalletConnect payloads, but raise it
to 10KB so real-world payloads fit. Update the error strings and the
inaccurate spec attributions.

Part of #957

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…d room

The 30% screen-height cap on the message area assumed the normal action
layout. Flagged requests add a security banner and stacked warning
buttons (~150pt extra), which could still push the actions off a small
screen.

Bound the sheet content to the usable window height and give the message
box flexShrink with a minimum scroll window, so the layout engine sizes
the message from the space actually remaining — banners and stacked
buttons now squeeze the message (which scrolls) instead of clipping the
action buttons.

Addresses Copilot review on #963.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings August 4, 2026 19:09

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes WalletConnect signing for long dApp messages while keeping controls visible.

Changes:

  • Raises the message limit from 1 KB to 10 KB.
  • Constrains and scrolls long message content.
  • Adds validation and display regression tests.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/providers/WalletKitProvider.tsx Clarifies message-limit validation.
src/helpers/walletKitValidation.ts Raises the limit to 10 KB.
src/i18n/locales/en/translations.json Updates the English limit message.
src/i18n/locales/pt/translations.json Updates the Portuguese limit message.
src/components/screens/WalletKit/DappSignMessageBottomSheetContent.tsx Bounds and shrinks sheet content.
src/components/screens/WalletKit/DappMessageDisplay.tsx Adds capped, shrinkable scrolling.
__tests__/helpers/walletKitValidation.test.ts Tests the increased limit.
__tests__/components/screens/WalletKit/DappMessageDisplay.test.tsx Tests display formatting and sizing.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

github-actions Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

iOS Simulator preview build is ready: https://github.com/stellar/freighter-mobile/releases/tag/untagged-61df1eb81e3e7ba182a5 (SDF collaborators only — install instructions in the release description)

@JakeUrban JakeUrban closed this Aug 6, 2026
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.

3 participants