[VPD-113]: add NativeTokenGateway compatible with BNB Core pool#8
[VPD-113]: add NativeTokenGateway compatible with BNB Core pool#8
Conversation
This NativeTokenGateway is compatible with Isolated Pools. It's not compatible with the BNB Chain Core pool. Source: https://github.com/VenusProtocol/isolated-pools/blob/58b76f81f15c61fc441bd1c5b2831509ac943288/contracts/Gateway/NativeTokenGateway.sol
Check the error codes returned by the VToke calls
chechu
force-pushed
the
feat/VPD-113
branch
from
d5f00bf to
83edda0
Compare
Debugger022
force-pushed
the
feat/VPD-113
branch
from
fc31938 to
381604a
Compare
deploy/native-token-gateway.ts
Outdated
| const { deployments, getNamedAccounts } = hre; | ||
| const { deploy } = deployments; | ||
| const { deployer } = await getNamedAccounts(); | ||
| const NormalTimelock = "0xce10739590001705F7FF231611ba4A48B2820327"; // BSC Testnet Normal Timelock |
There was a problem hiding this comment.
This must be recovered automatically, not hardcoded, depending on the selected network. We can read it from the npm governance package
| } | ||
|
|
||
| const VWNativeInfo: { [key: string]: VTokenConfig[] } = { | ||
| bsctestnet: [ |
There was a problem hiding this comment.
I would also add (not right now, but ideally before merging the PR) the configurations for the NTG of the Isolated Pools, on the rest of the networks. I would also copy/add the deployment files of those NTG contracts
deploy/native-token-gateway.ts
Outdated
| const VWNativeInfo: { [key: string]: VTokenConfig[] } = { | ||
| bsctestnet: [ | ||
| { | ||
| name: "vWBNB", |
There was a problem hiding this comment.
| name: "vWBNB", | |
| name: "vWBNB_Core", |
There was a problem hiding this comment.
I would add a number prefix to this deployment script. We won't follow a sequence of execution in this project, but I think it will help to clearly identify the files
There was a problem hiding this comment.
I would also consider the original fork tests in the IL repo, to check the NTG works as expected with IL markets
There was a problem hiding this comment.
I don't think we should mix tests and no-tests contracts in the same folder. Moreover, can we replace these imports with any configuration in hardhat?
There was a problem hiding this comment.
I don't think that we can replace Solidity import statements with Hardhat configuration, as it cannot substitute for actual Solidity imports in the source files.
| "chainId": "97", | ||
| "addresses": {} | ||
| "addresses": { | ||
| "NativeTokenGateway_vWBNB": "0xF34AAfc540Adc827A84736553BD29DE87a117558" |
There was a problem hiding this comment.
Should it be renamed to NativeTokenGateway_vWBNB_Core?
There was a problem hiding this comment.
the testnet VIP is already executed for this NTG address .
https://testnet.bscscan.com/tx/0x26abed53ac1d90a855d0b4c988894a6e3b9bd5c5c0cfae29158822a61c154ffe
There was a problem hiding this comment.
I don't see the dependency. It would be only to rename the deployment, not affecting the deployed code
|
There was a problem hiding this comment.
I think we don't need this file
deploy/02-native-token-gateway-il.ts
Outdated
There was a problem hiding this comment.
Do we need two deployment files? Cannot we use only one (taking into account that there will be only on NativeTokenGateway.sol file)
| "chainId": "97", | ||
| "addresses": {} | ||
| "addresses": { | ||
| "NativeTokenGateway_vWBNB": "0xF34AAfc540Adc827A84736553BD29DE87a117558" |
There was a problem hiding this comment.
I don't see the dependency. It would be only to rename the deployment, not affecting the deployed code
2 participants
This PR introduces NTG (NativeTokenGateway) for Core pool in bnb chain . NativeTokenGateway contract facilitates interactions with a vToken market for native tokens.