This repository was archived by the owner on Mar 5, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 5.1k
Fix typing compatibility when linking a contract to a context #5416
Merged
spacesailor24
merged 10 commits into
wyatt/4.x/5091-chainlink-plugin
from
fix/5091/change-web3context-generic-type
Oct 5, 2022
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
9cae1f9
fix typing compatibility when linking a contract to a context
Muhammad-Altabba 3e3cb8e
replace any with unknown at a suggested code for Web3Context
Muhammad-Altabba d43ac3a
Web3 net system tests (#5457)
nikoulai 6408b57
more fixes and sample calls for the plugin draft
Muhammad-Altabba d93c86c
tiny code revert for unused variable
Muhammad-Altabba 37cec51
add comments and replace some `any`s with `unknown`s for plugin relat…
Muhammad-Altabba 903d642
5427 contracts unit tests (#5465)
jdevcs b35c32f
Merge with 4.x
spacesailor24 4942447
Merge with 5091
spacesailor24 648a5c7
Merge branch 'wyatt/4.x/5091-chainlink-plugin' into fix/5091/change-w…
spacesailor24 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
34 changes: 34 additions & 0 deletions
34
packages/web3-eth-contract/test/fixtures/contracts/SampleStorageContract.sol
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| /* | ||
| This file is part of web3.js. | ||
|
|
||
| web3.js is free software: you can redistribute it and/or modify | ||
| it under the terms of the GNU Lesser General Public License as published by | ||
| the Free Software Foundation, either version 3 of the License, or | ||
| (at your option) any later version. | ||
|
|
||
| web3.js is distributed in the hope that it will be useful, | ||
| but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| GNU Lesser General Public License for more details. | ||
|
|
||
| You should have received a copy of the GNU Lesser General Public License | ||
| along with web3.js. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
| pragma solidity ^0.8.7; | ||
|
|
||
| contract SampleStorageContract { | ||
|
|
||
| uint256 uintNum; | ||
|
|
||
| event NEWNUM(uint256 param); | ||
|
|
||
| function storeNum(uint256 param) public { | ||
| uintNum = param; | ||
| emit NEWNUM(param); | ||
| } | ||
|
|
||
| function retrieveNum() public view returns (uint256){ | ||
| return uintNum; | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,59 @@ | ||
| /* | ||
| This file is part of web3.js. | ||
|
|
||
| web3.js is free software: you can redistribute it and/or modify | ||
| it under the terms of the GNU Lesser General Public License as published by | ||
| the Free Software Foundation, either version 3 of the License, or | ||
| (at your option) any later version. | ||
|
|
||
| web3.js is distributed in the hope that it will be useful, | ||
| but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| GNU Lesser General Public License for more details. | ||
|
|
||
| You should have received a copy of the GNU Lesser General Public License | ||
| along with web3.js. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
| // sample storage contract ABI | ||
| export const sampleStorageContractABI = [ | ||
| { | ||
| anonymous: false, | ||
| inputs: [ | ||
| { | ||
| indexed: false, | ||
| internalType: 'uint256', | ||
| name: 'param', | ||
| type: 'uint256', | ||
| }, | ||
| ], | ||
| name: 'NEWNUM', | ||
| type: 'event', | ||
| }, | ||
| { | ||
| inputs: [], | ||
| name: 'retrieveNum', | ||
| outputs: [ | ||
| { | ||
| internalType: 'uint256', | ||
| name: '', | ||
| type: 'uint256', | ||
| }, | ||
| ], | ||
| stateMutability: 'view', | ||
| type: 'function', | ||
| }, | ||
| { | ||
| inputs: [ | ||
| { | ||
| internalType: 'uint256', | ||
| name: 'param', | ||
| type: 'uint256', | ||
| }, | ||
| ], | ||
| name: 'storeNum', | ||
| outputs: [], | ||
| stateMutability: 'nonpayable', | ||
| type: 'function', | ||
| }, | ||
| ]; |
142 changes: 142 additions & 0 deletions
142
packages/web3-eth-contract/test/fixtures/unitTestFixtures.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,142 @@ | ||
| /* | ||
| This file is part of web3.js. | ||
|
|
||
| web3.js is free software: you can redistribute it and/or modify | ||
| it under the terms of the GNU Lesser General Public License as published by | ||
| the Free Software Foundation, either version 3 of the License, or | ||
| (at your option) any later version. | ||
|
|
||
| web3.js is distributed in the hope that it will be useful, | ||
| but WITHOUT ANY WARRANTY; without even the implied warranty of | ||
| MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the | ||
| GNU Lesser General Public License for more details. | ||
|
|
||
| You should have received a copy of the GNU Lesser General Public License | ||
| along with web3.js. If not, see <http://www.gnu.org/licenses/>. | ||
| */ | ||
|
|
||
| export const getLogsData = { | ||
| request: { | ||
| fromBlock: 'earliest', | ||
| toBlock: 'latest', | ||
| topics: ['0x7d7846723bda52976e0286c6efffee937ee9f76817a867ec70531ad29fb1fc0e'], | ||
| }, | ||
|
|
||
| response: [ | ||
| { | ||
| logIndex: 1, | ||
| transactionIndex: 0, | ||
| transactionHash: '0xbe70733bcf87282c0ba9bf3c0e2d545084fad48bd571c314140c8dc1db882673', | ||
| blockHash: '0x78755c18c9a0a1283fa04b2f78c7794c249395b08f7f7dff304034d64d6a1607', | ||
| blockNumber: 25, | ||
| address: '0x2D029a4bd792d795f35e0583F64eD9DedeBBa849', | ||
| data: '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000548656c6c6f000000000000000000000000000000000000000000000000000000', | ||
| topics: ['0x7d7846723bda52976e0286c6efffee937ee9f76817a867ec70531ad29fb1fc0e'], | ||
| type: 'mined', | ||
| id: 'log_886b29f0', | ||
| }, | ||
| ], | ||
| }; | ||
|
|
||
| export const getPastEventsData = { | ||
| event: 'GREETING_CHANGED', | ||
| response: [ | ||
| { | ||
| logIndex: BigInt(1), | ||
| transactionIndex: BigInt(0), | ||
| transactionHash: '0xbe70733bcf87282c0ba9bf3c0e2d545084fad48bd571c314140c8dc1db882673', | ||
| blockHash: '0x78755c18c9a0a1283fa04b2f78c7794c249395b08f7f7dff304034d64d6a1607', | ||
| blockNumber: BigInt(25), | ||
| address: '0x2D029a4bd792d795f35e0583F64eD9DedeBBa849', | ||
| data: '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000548656c6c6f000000000000000000000000000000000000000000000000000000', | ||
| topics: ['0x7d7846723bda52976e0286c6efffee937ee9f76817a867ec70531ad29fb1fc0e'], | ||
| returnValues: { | ||
| '0': 'Hello', | ||
| __length__: 1, | ||
| greeting: 'Hello', | ||
| }, | ||
| event: 'GREETING_CHANGED', | ||
| signature: '0x7d7846723bda52976e0286c6efffee937ee9f76817a867ec70531ad29fb1fc0e', | ||
| raw: { | ||
| data: '0x0000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000548656c6c6f000000000000000000000000000000000000000000000000000000', | ||
| topics: ['0x7d7846723bda52976e0286c6efffee937ee9f76817a867ec70531ad29fb1fc0e'], | ||
| }, | ||
| }, | ||
| ], | ||
| }; | ||
|
|
||
| export const AllGetPastEventsData = { | ||
| getLogsData: [ | ||
| { | ||
| logIndex: 0, | ||
| transactionIndex: 0, | ||
| transactionHash: '0x1ba478ce1810bfa8a0725c0ca94f3cfe163a70c396037a1f3c94cad34e497959', | ||
| blockHash: '0x79eece1fb22b7109f302b65bd826b1cebf9f704642e86ae9086ed93baf44a45e', | ||
| blockNumber: 20, | ||
| address: '0x20bc23D0598b12c34cBDEf1fae439Ba8744DB426', | ||
| data: '0x00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000548656c6c6f0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010416e6f74686572204772656574696e6700000000000000000000000000000000', | ||
| topics: ['0x0d363f2fba46ab11b6db8da0125b0d5484787c44e265b48810735998bab12b75'], | ||
| type: 'mined', | ||
| id: 'log_0a03b06c', | ||
| }, | ||
| { | ||
| logIndex: 1, | ||
| transactionIndex: 0, | ||
| transactionHash: '0x1ba478ce1810bfa8a0725c0ca94f3cfe163a70c396037a1f3c94cad34e497959', | ||
| blockHash: '0x79eece1fb22b7109f302b65bd826b1cebf9f704642e86ae9086ed93baf44a45e', | ||
| blockNumber: 20, | ||
| address: '0x20bc23D0598b12c34cBDEf1fae439Ba8744DB426', | ||
| data: '0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000010416e6f74686572204772656574696e6700000000000000000000000000000000', | ||
| topics: ['0x7d7846723bda52976e0286c6efffee937ee9f76817a867ec70531ad29fb1fc0e'], | ||
| type: 'mined', | ||
| id: 'log_389ae161', | ||
| }, | ||
| ], | ||
|
|
||
| response: [ | ||
| { | ||
| logIndex: BigInt(0), | ||
| transactionIndex: BigInt(0), | ||
| transactionHash: '0x1ba478ce1810bfa8a0725c0ca94f3cfe163a70c396037a1f3c94cad34e497959', | ||
| blockHash: '0x79eece1fb22b7109f302b65bd826b1cebf9f704642e86ae9086ed93baf44a45e', | ||
| blockNumber: BigInt(20), | ||
| address: '0x20bc23D0598b12c34cBDEf1fae439Ba8744DB426', | ||
| data: '0x00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000548656c6c6f0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010416e6f74686572204772656574696e6700000000000000000000000000000000', | ||
| topics: ['0x0d363f2fba46ab11b6db8da0125b0d5484787c44e265b48810735998bab12b75'], | ||
| returnValues: { | ||
| '0': 'Hello', | ||
| '1': 'Another Greeting', | ||
| __length__: 2, | ||
| from: 'Hello', | ||
| to: 'Another Greeting', | ||
| }, | ||
| event: 'GREETING_CHANGING', | ||
| signature: '0x0d363f2fba46ab11b6db8da0125b0d5484787c44e265b48810735998bab12b75', | ||
| raw: { | ||
| data: '0x00000000000000000000000000000000000000000000000000000000000000400000000000000000000000000000000000000000000000000000000000000080000000000000000000000000000000000000000000000000000000000000000548656c6c6f0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000010416e6f74686572204772656574696e6700000000000000000000000000000000', | ||
| topics: ['0x0d363f2fba46ab11b6db8da0125b0d5484787c44e265b48810735998bab12b75'], | ||
| }, | ||
| }, | ||
| { | ||
| logIndex: BigInt(1), | ||
| transactionIndex: BigInt(0), | ||
| transactionHash: '0x1ba478ce1810bfa8a0725c0ca94f3cfe163a70c396037a1f3c94cad34e497959', | ||
| blockHash: '0x79eece1fb22b7109f302b65bd826b1cebf9f704642e86ae9086ed93baf44a45e', | ||
| blockNumber: BigInt(20), | ||
| address: '0x20bc23D0598b12c34cBDEf1fae439Ba8744DB426', | ||
| data: '0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000010416e6f74686572204772656574696e6700000000000000000000000000000000', | ||
| topics: ['0x7d7846723bda52976e0286c6efffee937ee9f76817a867ec70531ad29fb1fc0e'], | ||
| returnValues: { | ||
| '0': 'Another Greeting', | ||
| __length__: 1, | ||
| greeting: 'Another Greeting', | ||
| }, | ||
| event: 'GREETING_CHANGED', | ||
| signature: '0x7d7846723bda52976e0286c6efffee937ee9f76817a867ec70531ad29fb1fc0e', | ||
| raw: { | ||
| data: '0x00000000000000000000000000000000000000000000000000000000000000200000000000000000000000000000000000000000000000000000000000000010416e6f74686572204772656574696e6700000000000000000000000000000000', | ||
| topics: ['0x7d7846723bda52976e0286c6efffee937ee9f76817a867ec70531ad29fb1fc0e'], | ||
| }, | ||
| }, | ||
| ], | ||
| }; |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APIshould be passed here otherwise the specification ofChainlinkPluginAPIis meaningless. The purpose of specifying theChainlinkPluginAPI(as we do with EthExecutionApi) is to provide type safety when using therequestManagere.g.APIspecifies whatmethods are available to call and their corresponding typedparamswhen doing things like so:which gives us type safety like so:
Removing
<API, RegisteredSubs>forlinkmeansAPIgets default tounknownas you've addedAPI extends Web3APISpec = unknown,which yields:which means TypeScript is no longer aware of what's available for the
requestManagerto call, whatparamsthe specifiedmethodtakes, and what the return type of the request isUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually,
requestManagerandlink<T extends Web3Context>are separate. And so not passingAPItolink<T extends Web3Context>does not affectrequestManager. And I kept it untouched as-is:And so the safe typing is working well:

And, if I provided no
paramstoeth_getBalancethen runningyarn buildwould give the following error:So, I think you just had a cashing problem when checking out this branch. Kindly try again by running
yarn build. And close vscode and reopen it, if it was still not showing you the correct error...And let me know, please, if there is any type-safe feature that is not working as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The third screenshot is the inferred type when using the
requestManagerfor the plugin. TheEthExecutionApi(i.e. therequestManagerwhen used within theweb3-ethpackage) still resolves as expected with your approach, but the sameAPItype resolution does not occur forChainlinkPluginAPI- this is what #5480 is attempting to solve. I don't think making use ofunknownis the best solution for this issue as we do know what theAPIis because we specify it. The changes in #5480 should allow us to pass anAPIthat's notEthExecutionApi(or extends it asWeb3EthExecutionApidoes), but as mentioned on our call and below, our code around subscriptions/logs is hardcoded to handleEthExecutionApiand therefore throws type errors when trying to use anAPIthat doesn't haveeth_subscribeThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the plugin writer would like to use
this._requestManager, then the plugin api would need to be something like:And then all the type-safety for the request manager would work inside the plugin.
However, I also I caught the source of confusion for
getPrice. This has nothing to do with theChainlinkPluginAPI. This should be only insideChainlinkPlugin. And, actually, when the user writesweb3.chainlink, it should be of the typeChainlinkPluginand notChainlinkPluginAPI.So, 2 places needed to be fixed for that:
First is the definition of the
chainlinkvariable:And is that the type of
ChainlinkPluginAPIshould contain only the JSON RPC methods for the connected node. So, ti should not containgetPrice. But if the node provides some non-standard JSON RPC method. Or JSON RPC methods for substrate, for example (if the plugin was expanded later for such a possibility).So, I pushed a commit that addresses those points in addition to a few enhancements....
Please, let me know if you have any other concerns about the provided solution. And if so, please provide a code snippet of what you would like to do exactly and I will help investigating that.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea I was totally thinking about
ChainlinkPluginAPIincorrectly - it's not needed given that there are no JSON RPC methods being added as you've mentioned. I think it's fine forWeb3PluginBaseto just default toWeb3APISpec(so the user doesn't have to worry about passing the generic if they're not adding a JSON RPC API) as added by this commitThe rest of the changes you've made in this PR (namely aroundI've also been thinking about this incorrectly, the issue I thought we had regarding instantiating aWeb3Context) I don't think are needed for #5393, but probably do need to be incorporated into #5480. #5480 is actually trying to solve an issue not strictly related to #5393 and that's fixing the existing architecture aroundWeb3Contextand theRequestManagerwhen you pass anAPIthat doesn't extendEthExecutionApi- which I don't believe should be a requirement when a plugin/user wants to add JSON RPC methods (it's currently a requirement since code around log subscriptions is currently hardcoded to useEthExecutionApias mentioned here)Web3Contextwith anAPIthat does not extendEthExecutionApiis non-existent as demonstrated by the following:The error I was actually seeing is fixed by extending
EthExecutionApilike you've mentioned above:Even though this
ChainlinkPluginAPIis not actually needed for the plugin (since we're not adding support for any new JSON RPC methods), for the sake of this example, let's just assume it's needed. When attempting to doYou receive the following error for the line
I was thinking
ChainlinkPluginAPIshouldn't need to extendEthExeuctionAPIin this case because it would add a bunch of JSON RPC methods that aren't going to be used for the plugin. However,ChainlinkPluginAPIneeds to extendEthExeuctionAPIbecause it's usingweb3-eth-contractwhich depends on JSON RPC methods defined byEthExeuctionAPIto perform certain actions. The error in my thinking was that you don't needEthExecutionAPIjust to call methods on a deployed contract (as the example Chainlink plugin is doing), but that would entail removing functionality from theweb3-eth-contractto get it to work which just doesn't make senseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great @spacesailor24 👍
I am happy to see that this MR helped resolve the 2 blockers for #5393 and provided clarifications 😄
And so it is time to merge this MR.
However, because I think that, there are some copied ideas from this MR to #5393 (and this is also the reason for having conflicts in the MR with the destination branch). I suggest you do one of the following:
rebaseyour branch to an earlier commit and then re-push the commits from this branch and from your branch in the order they came. And before doing this I recommend that you save the commits from both branches and the date of each one. And also to have a full copy of the local folder, that you can use in case things were messed-up. This will keep the contribution clean of who did what at which time and it would be easier to trace if later needed and this is the best practice, I think.And by the way, I also pushed some more enhancements and comments to the code so when someone later would implement a plugin would have a clearer understanding, I hope. And this included 2 classes
Web3PluginBaseandWeb3EthPluginBase😄.