Conversation
✅ Deploy Preview for testnet-hydra-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hydration-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for edge-hydra-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
jvonasek
force-pushed
the
feat/multisig-wallet
branch
4 times, most recently
from
56e960f to
f88f89b
Compare
jvonasek
force-pushed
the
feat/multisig-wallet
branch
7 times, most recently
from
a2a4c83 to
b1c3f82
Compare
| papi.constants.Multisig.DepositBase(), | ||
| papi.constants.Multisig.DepositFactor(), | ||
| ]) | ||
| const deposit = base + factor * BigInt(numSignatories) |
There was a problem hiding this comment.
use Big.js for calculations
There was a problem hiding this comment.
base and factor are native BigInt, using Big.js seems redundant just to convert it back to primitive type.
| const deposit = base + factor * BigInt(numSignatories) | ||
| return { | ||
| deposit, | ||
| depositHuman: scaleHuman(deposit, native.decimals), |
There was a problem hiding this comment.
dont' use scaleHuman anymore
| queryFn: async () => { | ||
| const [metadata, blockHash] = await Promise.all([ | ||
| hydration.getMetadata(), | ||
| papiClient._request<string>("chain_getBlockHash", [tx.when.height]), |
There was a problem hiding this comment.
why do not use the classic papi call?
There was a problem hiding this comment.
There doesn't seem to be a way to get block hash by block number
| ]) | ||
| const decoder = getExtrinsicDecoder(metadata) | ||
|
|
||
| const [body, timestamp] = await Promise.all([ |
There was a problem hiding this comment.
timestamp can be taken from query cache
There was a problem hiding this comment.
this is timestamp at specific block hash, not current timestamp
| enabled: isApiLoaded && !!signerAddress, | ||
| queryKey: ["multisig", "signerBalance", signerAddress], | ||
| queryFn: async () => { | ||
| const balanceData = await papi.query.System.Account.getValue( |
There was a problem hiding this comment.
Use sdk balance client instead
| @@ -0,0 +1,72 @@ | |||
| /* import { | |||
| (state) => state.pendingTransactions, | ||
| ) | ||
|
|
||
| const isMultisig = !!(account as StoredAccount | null)?.isMultisig |
There was a problem hiding this comment.
should type assertion be here?
| const otherSignatories = config.signers | ||
| .filter((s: string) => { | ||
| try { | ||
| const signerBytes = AccountId().enc(signerAddress) |
There was a problem hiding this comment.
signerBytes can be moved out from filter callback
| }), | ||
| } | ||
| }, | ||
| [account, activeMultisigConfig, papi], |
There was a problem hiding this comment.
you don't need the whole account object but only multisigSignerAddress. Can sort and filter be simplified without address encoding?
There was a problem hiding this comment.
Reworked, removed the custom sorting function and used helper from Papi, but it still has to be converted to raw Uint8Array
| } | ||
|
|
||
| export const MultisigStatus: FC<MultisigStatusProps> = ({ approved }) => { | ||
| const { t } = useTranslation() |
There was a problem hiding this comment.
add translation file here
| stringEquals( | ||
| safeConvertSS58toPublicKey(account?.multisigSignerAddress ?? ""), | ||
| entry.signatory.pubKey, | ||
| ) | ||
| const isConnected = | ||
| !!account && account.publicKey === entry.signatory.pubKey |
There was a problem hiding this comment.
just compare sigAddress (ss85) with ss58 with ss58 address, no need to convert them to public key, since ss58 is shorter. Also these two variables can be simplified to one
| {t("close")} | ||
| </Button> | ||
| </ModalCloseTrigger> | ||
| <Button |
| return keys.reduce<unknown>( | ||
| (acc, key) => | ||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
| acc === null || acc === undefined ? acc : prop(acc as any, key), |
There was a problem hiding this comment.
do we need this or it was generated by claud?
There was a problem hiding this comment.
Need this to parse wrapped multisig call, but I moved this to utils package and removed the TS ignore
| label={t("transaction.summary.cost.label")} | ||
| content={<ReviewTransactionFee />} | ||
| /> | ||
| {isMultisig && <MultisigDepositRow />} |
There was a problem hiding this comment.
just use one condition
There was a problem hiding this comment.
It has to be rendered separately because of Summary separator logic.
When using React.Fragment, it renders only one separator for both rows
| const { isApiLoaded, papi } = useRpcProvider() | ||
| const queryClient = useQueryClient() | ||
|
|
||
| const multisigs = useMemo( |
There was a problem hiding this comment.
do we need to memorize it?
|
|
||
| return ( | ||
| <SStepTrigger isInteractive={isDone} {...props}> | ||
| <Flex |
There was a problem hiding this comment.
create styled component
| const isSignerAddress = (address: string) => | ||
| config.signers.some((signer) => | ||
| stringEquals( | ||
| safeConvertSS58toPublicKey(signer), |
There was a problem hiding this comment.
why convert addresses?
There was a problem hiding this comment.
public key is unique, ss58 is not, we cannot be sure what ss58 format will be returned by Multix Indexer
| const indexedConfigs: MultisigConfig[] = | ||
| accounts | ||
| .filter((multisig) => isNumber(multisig.threshold)) | ||
| .filter((multisig) => { |
There was a problem hiding this comment.
can be moved to the first filter callback
| ), | ||
| isCustom: false, | ||
| } | ||
| }) ?? [] |
| { | ||
| name: "multisig-configs", | ||
| version: 2, | ||
| migrate: (persistedState: unknown, version: number) => { |
There was a problem hiding this comment.
you don't need I guess, since that's a new store and version can be set to 0 or 1
| // explicit account → null transition, not on every unrelated state update | ||
| // (e.g. wallet connecting / accounts array loading). | ||
| /* const prevAccountRef = useRef(useWeb3Connect.getState().account) | ||
| useEffect(() => { |
jvonasek
force-pushed
the
feat/multisig-wallet
branch
from
b1c3f82 to
43b9742
Compare
jvonasek
force-pushed
the
feat/multisig-wallet
branch
from
43b9742 to
9842133
Compare
2 participants
implements #3689