add batchers + build functions #10
Conversation
.eslintrc.js
Outdated
| "ecmaVersion": "latest", | ||
| "sourceType": "module" | ||
| }, | ||
| "rules": { |
There was a problem hiding this comment.
one rule I feel like is practically indespensible is no hanging promises https://github.com/usecannon/cannon/blob/main/.eslintrc.json#L55
package.json
Outdated
| @@ -5,7 +5,8 @@ | |||
| "main": "dist/src/index.js", | |||
| "scripts": { | |||
| "build": "forge build && tsc", | |||
| "test": "node ./test/pyth.mjs" | |||
| "test": "node ./test/pyth.mjs", | |||
| "lint:fix": "eslint . --ext .ts --fix" | |||
There was a problem hiding this comment.
non fix command would also be useful here
| to: address, | ||
| data: viem.encodeFunctionData({ | ||
| abi: ISmartAccount.abi, | ||
| functionName: 'executeBatch', |
There was a problem hiding this comment.
so the biggest problem with this method of determining batcher is that the on-chain status of the contract we are sending the txn from should not be the method by which we determine what is supported/not supported in terms of batching. additionally, these types of calls are going to introduce significant lag and unnecessary on-chain calls as we add more supported wallets.
instead, the way we should determine the batching method should be detected by the wallet that the user/selects/connects to their session. For example, if the user connects "Biconomy" wallet, the frontend can be aware of this and report it to the library. Alternatively, the viem.WalletClient can be checked for functionality (ex. if it responds to the erc4336 calls)
if the wallet is not detected, we can always fallback to attempting to do it with trusted forwarder (which can be configured by the frontend as needed)
src/index.ts
Outdated
| tx: TransactionRequest | TransactionRequest[] | ||
| transactions: TransactionRequest | TransactionRequest[] | ||
| ): Promise<TransactionRequest[]> { | ||
| return await (this.enableERC7412(client, transactions, true) as Promise< |
There was a problem hiding this comment.
needing to cast to Promise<TransactionRequest[]> here (as you imply in your comment) is not a good idea. something seems wrong with this. investigation required.
src/index.ts
Outdated
| ): Promise<TransactionRequest> { | ||
| const multicallCalls: TransactionRequest[] = Array.isArray(tx) ? tx : [tx]; | ||
| const batcher = this.batchers.find( | ||
| async (batch) => await batch.batchable(client, transactions) |
There was a problem hiding this comment.
cant use async in javascript find function.
Instead, use Promise.all and find from that result
There was a problem hiding this comment.
and it should go without saying this is extremely inefficient and slow, especially as more wallets are added. it would be better to detect which wallet the user is using by inspecting the wallet that was chosen by their picker, or testing the RPC methods available
src/index.ts
Outdated
| } | ||
|
|
||
| const signedRequiredData = await adapter.fetchOffchainData( | ||
| client, | ||
| oracleAddress, | ||
| oracleQuery | ||
| ); | ||
| ) | ||
|
|
||
| const priceUpdateTx = { |
There was a problem hiding this comment.
this var should probably be renamed to somethign like offchainDeliveryTx
src/index.ts
Outdated
| args: [signedRequiredData] | ||
| }) | ||
| } | ||
| multicallCalls.unshift(priceUpdateTx as TransactionRequest) |
There was a problem hiding this comment.
what casting is needed here? are fields missing? is something possibly null? probably best not to cast here unless its really needed.
|
ftr I am going to do some work on this branch |
this first build focuses on getting really good read function support. this is accomplished a few ways: * have a few simple functions which allow for doing the most basic operations in * havk a higher level function `callWithOffchainData` which can be used to instantly add erc7412 to any `call` effective read request * include a viem extension which can allow for any public provider to be automatically augmented to be erc7412 read compatible
feat: update client
No description provided.