Add read methods #6
Conversation
| * @param ethers - Ethers v5 library | ||
| * @param signer - Ethers signer | ||
| * @param safeAddress - The address of the Safe account to use | ||
| * @returns The Safe Core SDK instance |
There was a problem hiding this comment.
shouldn't return also include a type? https://jsdoc.app/tags-returns.html
btw I think typescript supports jsdoc and can generate types from it, so maybe we don't need to duplicate them: https://www.typescriptlang.org/docs/handbook/jsdoc-supported-types.html#param-and-returns
README.md
Outdated
| const wallet2 = ethers.Wallet.createRandom().connect(provider) | ||
|
|
||
| const safe = await safeContract.setup( |
There was a problem hiding this comment.
Where is the safeContract coming from?
There was a problem hiding this comment.
How would you represent an existing Safe account here? It doesn't make sense to me to add all the logic to explain where this is coming from.
There was a problem hiding this comment.
It is also weird that a direct interaction with the contract is needed before using the SDK. I guess at some point the SDK should allow to create Safes, right? Should this be prioritized?
There was a problem hiding this comment.
maybe just add a comment like
// Existing Safe address (e.g. create from a Safe created via https://app.gnosis-safe.io
const safeAddress = "0x<some_safe_address>"
src/EthersSafe.ts
Outdated
| * @param hash - The data to sign | ||
| * @returns The Safe signature | ||
| */ | ||
| async signMessage(hash: string): Promise<SafeSignature> { |
There was a problem hiding this comment.
I am not sure about the name as the Safe also has a method called signMessage
There was a problem hiding this comment.
Is signTransactionHash better?
There was a problem hiding this comment.
yes, this would make it a lot clearer for me :)
src/utils/transactions.ts
Outdated
| @@ -49,6 +49,12 @@ export class SafeTransaction { | |||
| } | |||
| } | |||
|
|
|||
| /** | |||
There was a problem hiding this comment.
the baseOfsset in line 40 should probably be signers.length * 65 as we need the offset in bytes not in chars.
Add read methods to access the data from the Safe contract