Ethers refactor #100
Ethers refactor #100
Conversation
Codecov Report
@@ Coverage Diff @@
## master #100 +/- ##
==========================================
- Coverage 92.38% 91.11% -1.28%
==========================================
Files 35 35
Lines 788 754 -34
==========================================
- Hits 728 687 -41
- Misses 60 67 +7
... and 5 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
|
|
||
| const decodeData = ethersInterface.decodeFunctionResult(node.callee, data) | ||
|
|
||
| return new TypedValue(returnType, decodeData[selectedReturnValueIndex]) |
There was a problem hiding this comment.
Not opposed to the above and being low-level, but I wonder if creating an ethers contract wouldn't be simpler? Effectively we've created an ABI for the interface, and so it seems like something that could just go into the contract object.
There was a problem hiding this comment.
After trying to use the ethers Contract object I found that in this particular case working with the Interface object was easier because we need to select a specific return value element.
| * @return {Promise<radspec/evaluator/TypedValue>} | ||
| */ | ||
| async (addr, data) => { | ||
| async (addr, data, registryAddress) => { |
There was a problem hiding this comment.
What do you think of this optional registry address to set up the Method Registry?
| } else { | ||
| } catch { | ||
| // Try fetching 4bytes API | ||
| const { results } = await ethers.utils.fetchJson( |
There was a problem hiding this comment.
If we can't resolve the signature with the on-chain registry we try fetching the 4bytes API. Otherwise, we finally default to an unknown function.
test/examples/examples.js
Outdated
| source: 'Perform action: `@radspec(addr, data)`', | ||
| bindings: { | ||
| addr: address(), | ||
| data: bytes('0x0b30a8d7') // getLocators(address,uint256), from 4bytes api |
There was a problem hiding this comment.
Test random signature from 4bytes API.
| : (await provider.getNetwork()).chainId | ||
| }) | ||
| const result = await registry.lookup(methodId) | ||
| const { name } = parse(result) | ||
| return { | ||
| type: 'string', | ||
| value: name // TODO: should we decode and print the arguments as well? |
There was a problem hiding this comment.
I wonder what you think about this TODO 🤔
|
I spent quite some time trying to migrate the I saw this issue that discuss a possible solution. But then I further look into the documentation and it seems humanize might not be the right way to convert between units. The way suggested is to use |
|
We finally decide to stick with |
|
UPDATE: Add a timeout to request 4bytes API. An example of it using Aragon Connect, the request takes about 1 minute: https://stackblitz.com/edit/aragon-connect-uprtcl |
This PR refactor radspec logic to use
ethers.jslibrary instead of several packages ofweb3andbn.js.The changes are structure in four commits:
src.Note we are using the latest ethers beta version, the documentation site might be helpful to better understand part of the logic. In specific how the Provider, Interface, and Fragments objects work: