Spaceward: use MsgNewAction for creating new Actions #385
Conversation
|
Warning Review failedThe pull request is closed. WalkthroughWalkthroughThe overall changes involve removing the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant QueryClient
participant ActionHandler
participant BigIntUtil
User->>App: Interact with UI
App->>QueryClient: Initialize with hashQueryKey
App->>ActionHandler: Trigger action
ActionHandler->>BigIntUtil: Convert ID using BigInt
BigIntUtil-->>ActionHandler: Return BigInt ID
ActionHandler-->>App: Return action result
App-->>User: Display result
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
|
Hey @Pitasi and thank you for opening this pull request! 👋🏼 It looks like you forgot to add a changelog entry for your changes. Make sure to add a changelog entry in the 'CHANGELOG.md' file. |
40ab076 to
4b952e8
Compare
There was a problem hiding this comment.
Actionable comments posted: 6
Outside diff range and nitpick comments (5)
spaceward/src/utils/any.ts (1)
4-15: Well implemented utility functions for handling Protocol BuffersAnymessages.Consider adding detailed documentation for the
Msginterface andpackAnyfunction to improve code readability and maintainability.spaceward/src/hooks/useModuleAccount.ts (1)
4-14: Well implemented hook for retrieving module accounts.Consider adding error handling for cases where the
cosmos.auth.v1beta1.useModuleAccountscall fails or returns unexpected results.spaceward/src/hooks/useEthereumTx.tsx (1)
17-17: The update to usebigintinsignEthereumTxis correctly implemented.Consider adding more detailed logging for debugging purposes, especially when the
ethereumAnalyzerContractis missing.spaceward/src/hooks/useAction.ts (1)
9-30: Well implementeduseNewActionhook for creating new actions.Consider adding unit tests for the
useNewActionfunction to ensure its functionality and handle edge cases.spaceward/src/hooks/useClient.ts (1)
Line range hint
57-119: Review the use of@ts-ignoreto suppress TypeScript errors. It's generally better to handle the types explicitly unless there's a compelling reason to suppress the errors.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (25)
- spaceward/.eslintrc.cjs (1 hunks)
- spaceward/package.json (1 hunks)
- spaceward/pnpm-lock.yaml (2 hunks)
- spaceward/src/App.tsx (1 hunks)
- spaceward/src/components/SendEth.tsx (2 hunks)
- spaceward/src/features/assets/Assets.tsx (7 hunks)
- spaceward/src/features/home/HomeAssets.tsx (5 hunks)
- spaceward/src/features/home/TotalAssetValue.tsx (2 hunks)
- spaceward/src/features/keys/Keys.tsx (3 hunks)
- spaceward/src/features/keys/NewKeyButton.tsx (3 hunks)
- spaceward/src/features/metamask/AddToMetaMaskButton.tsx (2 hunks)
- spaceward/src/features/metamask/MetaMaskRequests.tsx (6 hunks)
- spaceward/src/features/owners/AddSpaceOwnerForm.tsx (3 hunks)
- spaceward/src/features/spaces/SpaceSelector.tsx (5 hunks)
- spaceward/src/features/walletconnect/WalletConnect.tsx (3 hunks)
- spaceward/src/hooks/useAction.ts (1 hunks)
- spaceward/src/hooks/useClient.ts (2 hunks)
- spaceward/src/hooks/useEthereumTx.tsx (2 hunks)
- spaceward/src/hooks/useModuleAccount.ts (1 hunks)
- spaceward/src/hooks/useRequestKey.ts (4 hunks)
- spaceward/src/hooks/useRequestSignature.ts (5 hunks)
- spaceward/src/pages/Intents.tsx (4 hunks)
- spaceward/src/pages/Owners.tsx (2 hunks)
- spaceward/src/utils/any.ts (1 hunks)
- spaceward/src/utils/queryKeyHash.ts (1 hunks)
Files not summarized due to errors (1)
- spaceward/src/features/walletconnect/WalletConnect.tsx: Error: Server error. Please try again later.
Files skipped from review due to trivial changes (3)
- spaceward/.eslintrc.cjs
- spaceward/package.json
- spaceward/pnpm-lock.yaml
Additional comments not posted (35)
spaceward/src/hooks/useEthereumTx.tsx (1)
8-8: The update to usebigintinsignRawis correctly implemented.spaceward/src/features/owners/AddSpaceOwnerForm.tsx (3)
6-7: IntroduceduseNewActionandwardenimports to support the new action creation mechanism.Also applies to: 9-9
11-12: Refactored to useuseNewActionfor creating new space owners, aligning with the new action-based architecture.
71-71: The button's onClick handler correctly resets thenewOwnerstate after attempting to add an owner.spaceward/src/utils/queryKeyHash.ts (5)
1-9: Introduced types and thehashQueryKeyfunction for handling and hashing query keys.
12-26: ThestableValueHashfunction uses a custom serializer for JSON.stringify to handlebigintand plain objects, ensuring consistent hashing.
28-38: Added utility functionsisBigIntandensureQueryKeyArrayto support the hashing process.
40-67: ImplementedisPlainObjectto check for plain JavaScript objects, useful for serialization and hashing.
69-72: ThehasObjectPrototypefunction assists in determining if an object is a plain JavaScript object, supporting theisPlainObjectchecks.spaceward/src/features/metamask/AddToMetaMaskButton.tsx (1)
14-14: Updated thekeyIdparameter to usebigint, aligning with the new type standards in the project.spaceward/src/pages/Owners.tsx (2)
8-9: IntroduceduseNewActionand related imports to manage space owner actions, aligning with the new action-based architecture.Also applies to: 11-11
71-71: The button's onClick handler correctly triggers theremoveOwnerfunction. TheAddSpaceOwnerFormis correctly integrated with thespaceId.Also applies to: 79-79
spaceward/src/features/home/TotalAssetValue.tsx (1)
39-42: Ensure that theBigIntconversion is safe and won't cause runtime errors whenspaceIdis an empty string.#!/bin/bash # Description: Verify that `spaceId` is always a valid number before conversion to `BigInt`. # Test: Search for any assignments to `spaceId` that might not ensure a valid number. ast-grep --lang javascript --pattern $'const { spaceId } = useSpaceId(); $$$'spaceward/src/features/keys/NewKeyButton.tsx (2)
117-120: LGTM! The conversion toBigIntin theonClickhandler is correctly implemented.
33-33: Ensure that theuseQueryHooksis properly implemented and tested, especially thewarden.warden.v1beta2.useKeychainsmethod.#!/bin/bash # Description: Verify the implementation of `useQueryHooks` and its methods. # Test: Search for the implementation of `useQueryHooks`. ast-grep --lang javascript --pattern $'export function useQueryHooks() { $$$ }'Also applies to: 43-44
spaceward/src/hooks/useRequestKey.ts (1)
9-11: Ensure that the newuseNewActionhook is robustly implemented and tested, especially in handling errors and edge cases.#!/bin/bash # Description: Verify the implementation of `useNewAction` and its error handling. # Test: Search for the implementation of `useNewAction`. ast-grep --lang javascript --pattern $'export function useNewAction() { $$$ }'Also applies to: 26-53, 60-120
spaceward/src/features/spaces/SpaceSelector.tsx (1)
46-47: Ensure correct usage ofBigIntfor IDs.The conversion of
signIntentIdandadminIntentIdtoBigIntaligns with the PR's objective to handle large integers more efficiently. This change is crucial for maintaining data integrity and avoiding potential overflow issues with large numbers.spaceward/src/features/keys/Keys.tsx (1)
29-35: Updated parameter types toBigInt.The update from
LongtoBigIntforspaceIdandlimitin theuseKeysBySpaceIdfunction is a positive change. It ensures that the application can handle larger numbers, which is essential for scalability and robustness.spaceward/src/components/SendEth.tsx (1)
53-53: Conversion toBigIntfor key ID.The change to use
BigIntfor theidparameter in theuseKeyByIdfunction is consistent with the overall PR goal of replacingLongwithBigInt. This ensures that the application can handle larger integers effectively, which is particularly important for cryptographic operations and identifiers.spaceward/src/hooks/useRequestSignature.ts (2)
39-51: Refactored to useuseNewActionfor creating new signature requests.The refactoring to use the
useNewActionhook for creating new signature requests simplifies the code and aligns with the PR's objectives to enhance modularity. This change also potentially improves the maintainability of the code by centralizing the action creation logic.
Line range hint
58-74: Handling of empty arrays and use ofBigInt.The handling of potentially empty arrays by converting them to
undefinedis a clever workaround for issues with generated code not handling empty arrays correctly. Additionally, the consistent use ofBigIntforkeyIdensures that the application can handle larger integers, aligning with the PR's goals.spaceward/src/App.tsx (2)
60-60: Good integration ofhashQueryKeyinto the query client configuration. This should enhance the cache management by providing a consistent hashing mechanism for query keys.
54-54: Ensure that thehashQueryKeyfunction is correctly implemented and tested since it's now a crucial part of the query configuration.#!/bin/bash # Description: Verify the implementation and test coverage for `hashQueryKey`. # Test: Search for the function implementation and tests. Expect: Implementation and tests exist. rg --type typescript $'hashQueryKey'spaceward/src/features/home/HomeAssets.tsx (3)
29-35: UsingBigIntforspaceIdandlimitin the query setup is a good practice for handling large numbers, ensuring the application can scale without issues related to number size.
69-69: Correctly updated the key attribute to usetoString()onBigInt. This ensures that React can handle the key as a string, which is necessary for keys in lists.
101-101: Updating thekeyIdparameter tobigintaligns with the changes in the rest of the application to handle large numbers. Ensure all usages ofAddressare updated accordingly.#!/bin/bash # Description: Verify all usages of `Address` component have updated `keyId` parameter. # Test: Search for `Address` component usage. Expect: All usages have `keyId` as `bigint`. rg --type typescript $'Address'spaceward/src/features/assets/Assets.tsx (3)
42-48: UsingBigIntforspaceIdandlimitin the query setup is consistent with the changes across the application to handle large numbers effectively.
78-78: Properly updated the key attribute to usetoString()onBigInt, ensuring React can handle the key as a string. This is important for list rendering.
120-120: The update of thekeyIdparameter tobigintis necessary for consistency with the rest of the application's handling of large numbers.spaceward/src/features/metamask/MetaMaskRequests.tsx (4)
Line range hint
23-34: Introduction of theSignTransactionParamsinterface helps in defining a clear contract for transaction parameters, which is crucial for maintaining type safety and clarity in function calls.
95-95: Correctly handlingkeyIdasBigIntin thehandleApproveRequestfunction ensures that large numbers are managed properly throughout the application.
130-130: Proper handling of transaction parameters using theSignTransactionParamstype in theeth_signTransactioncase ensures type safety and clarity.
157-157: Ensuring that the typed message parameters are handled correctly in theeth_signTypedData_v4case is crucial for the integrity of the signing process.spaceward/src/pages/Intents.tsx (2)
15-16: ImporteduseNewActionandwardenfor new action creation and handling.This aligns with the PR's objective to utilize
MsgNewActionfor creating new actions, enhancing the modularity and maintainability of the code.
131-132: IntroducedMsgUpdateSpacefromwarden.warden.v1beta2.This change is necessary for the new action creation process and ensures that the correct message type is used.
The current state of wardenjs + Keplr/Leap/... doesn't play well with BigInts, when using Amino. Switch back to Protobuf encoding for better support.
This hook is convenient for sending new MsgNewAction transactions.
4b952e8 to
e499063
Compare
|
sorry @jjheywood, had to fix some conflicts and dismissed your approval |
Bump wardenjs to use BigInts instead of Longs (see #384), and use a new hook called
useNewAction()to create and broadcast transactions with aMsgNewActionmessage.This affects: MsgUpdateSpace, MsgAddSpaceOwner, MsgRemoveSpaceOwner, MsgUpdateKey (unused in Spaceward), MsgNewKeyRequest, MsgNewSignatureRequest.
I think I fixed all usages.
I had to switch back from Amino to Protobuf message encoding, unfortunately, because I couldn't get Keplr/wardenjs to play well with the fact that MsgNewAction has a
Anyfield. I'm in contact with the Telescope team to figure this out.Summary by CodeRabbit
New Features
useNewActionfor creating new actions and transactions.Anymessages.Refactor
LongwithBigIntfor handling numeric values across multiple components.useNewAction.Chores
"long": "^5.2.3".'plugin:storybook/recommended'from ESLint configuration.