Skip to content

fix: add Authorizations section on SignTransaction and SignAuthEntry dialog - #173

Merged
stanleyyconsensys merged 4 commits into
mainfrom
fix/sign-dialog
Aug 6, 2026
Merged

fix: add Authorizations section on SignTransaction and SignAuthEntry dialog#173
stanleyyconsensys merged 4 commits into
mainfrom
fix/sign-dialog

Conversation

@stanleyyconsensys

@stanleyyconsensys stanleyyconsensys commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Explanation

Adds a dedicated Authorizations section to the Snap confirmation UIs for SignTransaction and SignAuthEntry, and centralizes Soroban invocation rendering so both dialogs share the same layout.

Changes:

  • Introduces shared UI components for Soroban invocation + JSON argument rendering and reuses them in both confirmation views.
  • Extends transaction mapping to extract Soroban authorization entries into a separate readableTransaction.authorizations structure.
  • Adds getParam helper (with tests) and updates i18n strings to support the new section.

Sign Auth Entry
https://www.loom.com/share/80a6c678019f4e0291eecd67b25a980a

Sign Transaction
https://www.loom.com/share/3acce44e054c4b8f8306967e17925636

References

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've communicated my changes to consumers by updating changelogs for packages I've changed
  • I've introduced breaking changes in this PR and have prepared draft pull requests for clients and consumer packages to resolve them

* @param authEntry - Base64-encoded `HashIdPreimage` XDR.
* @returns Fields displayed in the confirmation dialog.
*/
function decodeSorobanAuthPreimage(authEntry: string): ReadableAuthEntry {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

remove all redundant function, as operationMapper now support the mapping

const preimage = xdr.HashIdPreimage.fromXDR(authEntry, 'base64');
const sorobanAuth = preimage.sorobanAuthorization();

let functionType: ReadableInvocation['functionType'];

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

remove all redundant function, as operationMapper now support the mapping

/**
* Abstract class for mapping operations to readable JSON.
*/
class AbstractOperationMapper {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

new class to share field and normalizeAmount function

}
}

export class AuthorizationMapper extends AbstractOperationMapper {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

new class to handle mapping for authorization

feeSourceAccount: transaction.feeSourceAccount,
memo: transaction.getMemo(),
operations,
authorizations,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

add authorizations field for UI to handle

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

Adds a dedicated Authorizations section to the Snap confirmation UIs for SignTransaction and SignAuthEntry, and centralizes Soroban invocation rendering so both dialogs share the same layout.

Changes:

  • Introduces shared UI components for Soroban invocation + JSON argument rendering and reuses them in both confirmation views.
  • Extends transaction mapping to extract Soroban authorization entries into a separate readableTransaction.authorizations structure.
  • Adds getParam helper (with tests) and updates i18n strings to support the new section.

Reviewed changes

Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
packages/snap/src/ui/confirmation/views/ConfirmSignTransaction/ConfirmSignTransaction.tsx Renders new Authorizations section and uses shared invocation/params summaries.
packages/snap/src/ui/confirmation/views/ConfirmSignAuthEntry/ConfirmSignAuthEntry.tsx Switches sign-auth-entry UI to render Authorizations section and shows nonce/expiry rows.
packages/snap/src/ui/confirmation/utils.ts Adds getParam helper for extracting fields from readable params.
packages/snap/src/ui/confirmation/utils.test.ts Adds unit tests for getParam.
packages/snap/src/ui/confirmation/components/JsonParamsSummary.tsx New component to render JSON/array params as copyable, labeled argument rows.
packages/snap/src/ui/confirmation/components/InvocationSummary.tsx New shared component for Soroban invocation (contract/function/args).
packages/snap/src/ui/confirmation/components/index.ts Exports newly added confirmation UI components.
packages/snap/src/ui/confirmation/components/Authorizations.tsx New component to display a top-level Authorizations section in confirmations.
packages/snap/src/services/transaction/OperationMapper.ts Adds authorization mapping + refactors field normalization and field typing via FieldType.
packages/snap/src/services/transaction/OperationMapper.test.ts Adds coverage for mapping invokeHostFunction auth entries into the new authorizations structure.
packages/snap/src/handlers/keyring/signAuthEntry.ts Refactors SEP-43 preimage decoding to reuse AuthorizationMapper and new readable shape.
packages/snap/src/handlers/keyring/signAuthEntry.test.ts Updates expectations to match the new readable auth-entry structure.
packages/snap/messages.json Updates/renames confirmation strings for new Authorizations + Invocation labels.
packages/snap/locales/en.json Mirrors messages updates for English locale.
packages/snap/locales/es.json Mirrors messages updates for Spanish locale.

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

Comment on lines +55 to +57
<Box>{null}</Box>
<Divider />
<Box>{null}</Box>

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

this is by purpose to make space

Comment thread packages/snap/src/ui/confirmation/components/JsonParamsSummary.tsx Outdated
'note',
`Unhandled or newer operation type "${unknownOp.type}".`,
'text',
`Unhandled or newer operation type.`,

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we dont need to show the type

Comment thread packages/snap/src/ui/confirmation/utils.ts Outdated
Comment thread packages/snap/src/ui/confirmation/components/JsonParamsSummary.tsx Outdated
// (whose children we expand into their own card right below) and enabled
// only for deeper nesting where we don't recurse — there the count is the
// only signal the user gets that more calls exist beneath.
const InvocationSummary = ({

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

move it to share component

{showNestedCount && subInvocations.length > 0 ? (
<Box direction="vertical">
<SnapText fontWeight="medium" color="alternative">
{translate('confirmation.signAuthEntry.subInvocations')}

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

we dont need to display the length of subInvocations

align with sign txn

* @param params.translate - Translation function.
* @returns JSX for the confirmation row value.
*/
const JsonParamValue = ({

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

move it to share JsonParamsSummary

case 'json':
return <JsonParamValue value={value} translate={translate} />;
default:
if (Array.isArray(value)) {

@stanleyyconsensys stanleyyconsensys Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

either json, copyable, array, string or object, it already handle in JsonParamsSummary

Comment on lines +345 to +346
{isInvokeHostFunction ? (
<InvocationSummary

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.

Should this also handle the note and source fields, or do those intentionally not apply here?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

good catch

@jeremytsng
jeremytsng self-requested a review August 6, 2026 10:08
@stanleyyconsensys
stanleyyconsensys merged commit be037bc into main Aug 6, 2026
10 checks passed
@stanleyyconsensys
stanleyyconsensys deleted the fix/sign-dialog branch August 6, 2026 10:12
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