-
Notifications
You must be signed in to change notification settings - Fork 0
SOV-5223: tx dialog #14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for sovryn-layer ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR implements a transaction dialog system (SOV-5223) that provides a unified interface for handling blockchain transactions, message signing, and typed data signing. The changes introduce a new transaction request abstraction layer in the SDK and a React-based dialog component for managing transaction workflows.
Key changes:
- New transaction request types and helper functions in the SDK to support messages, typed data, and transactions
- Transaction dialog component with state management using Zustand
- Updated Money Market manager to use the new transaction request system
- Enhanced network configuration to support multiple chains
Reviewed Changes
Copilot reviewed 18 out of 21 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/sdk/src/types.ts | Added generic type parameter to TransactionOpts and converted BorrowRateMode enum to const object with bigint values |
| packages/sdk/src/managers/money-market.manager.ts | Refactored borrow method to return new SdkTransactionRequest array, added test transaction/signature requests |
| packages/sdk/src/lib/transaction.ts | Created new transaction request abstraction with types and helper functions for messages, typed data, and transactions |
| packages/sdk/src/lib/helpers.ts | Added toAddress helper function to convert Account or Address to lowercase Address |
| packages/sdk/src/index.ts | Exported transaction.ts module |
| nx.json | Added build-deps dependency to dev target |
| eslint.config.mjs | Disabled @typescript-eslint/no-explicit-any rule |
| apps/web-app/src/lib/transactions/store.ts | Created Zustand store for managing transaction dialog state |
| apps/web-app/src/lib/transactions/index.ts | Created useSlayerTx hook for initiating transaction flows |
| apps/web-app/src/integrations/wagmi/root-provider.tsx | Replaced inline Tx component with TransactionDialogProvider |
| apps/web-app/src/integrations/wagmi/config.ts | Added support for multiple chains (mainnet, sepolia, rootstock, etc.) |
| apps/web-app/src/integrations/privy/root-provider.tsx | Added multiple chain support to Privy configuration |
| apps/web-app/src/integrations/privy/connector.tsx | Refactored to use new transaction dialog system |
| apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx | Implemented full-featured transaction dialog with wallet integration |
| apps/web-app/src/components/MoneyMarket/components/BorrowAssetsList/components/AssetsTable/AssetsTable.tsx | Updated to use new transaction dialog system |
| apps/web-app/public/locales/en/tx.json | Added transaction dialog translations |
| apps/web-app/public/locales/en/common.json | Added common action translations |
| apps/web-app/package.json | Added zustand, immer, use-sync-external-store dependencies and updated viem/wagmi versions |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 18 out of 21 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
Comments suppressed due to low confidence (1)
packages/sdk/src/managers/money-market.manager.ts:1
- The magic number '1' is used for the rate mode parameter. This should use the exported
borrowRateModes.stableconstant instead for better code clarity and maintainability.
import { Decimal } from '@sovryn/slayer-shared';
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/TransactionDialog.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 25 out of 27 changed files in this pull request and generated 6 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
apps/web-app/src/components/TransactionDialog/hooks/use-internal-tx-handler.ts
Show resolved
Hide resolved
apps/web-app/src/components/TransactionDialog/hooks/use-internal-tx-handler.ts
Show resolved
Hide resolved
| import type { Address, Chain, Hash } from 'viem'; | ||
| import { useChains } from 'wagmi'; | ||
|
|
||
| type ChainProps = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Create LinkToExplorer.types.ts and put the type definitions there (except LinkToExplorerProps).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think moving it to another file just increases compilation time but does not add any benefit as these types aren't supposed to be used anywhere else atm
https://sovryn.atlassian.net/browse/SOV-5223