Add Multisig #433
Add Multisig #433
Conversation
|
Test project (including only Multisig contracts) compiled successfully with 
|
| } | ||
|
|
||
| export interface MultisigOptions extends CommonOptions { | ||
| name: string; |
There was a problem hiding this comment.
The name is configurable from the API but not from the UI. Would there be any benefit to allow customizing the name in the UI?
(I realize this is also the case for VestingWallet)
andrew-fleming
left a comment
andrew-fleming
left a comment
There was a problem hiding this comment.
LGTM! I left a small comment
packages/core-cairo/src/multisig.ts
Outdated
|
|
||
| export const numberPattern = /^(?!$)(\d*)(?:\.(\d+))?(?:e(\d+))?$/; |
There was a problem hiding this comment.
Small thing: we could move this to somewhere in utils/ to avoid repetition since we're also using it in governor
There was a problem hiding this comment.
This regex looks like it should support decimals and e notation, although those give an error in the UI. I'm not sure why that is the case.
Regardless, perhaps the regex should be updated to only look for natural numbers.
|
👍 Dependency issues cleared. Learn more about Socket for GitHub ↗︎ This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
|
@ericnordelo @ericglau |
5 participants
No description provided.