-
Notifications
You must be signed in to change notification settings - Fork 20
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: chainbridge linked to new binary for CI test #1376
Conversation
@@ -123,9 +123,9 @@ async function setupCrossChainTransfer( | |||
await eConfig.erc20.mint(eConfig.wallets.eve.address, toWei('100000')); | |||
await eConfig.erc20.mint(eConfig.erc20Handler.address, toWei('300')); | |||
await eConfig.bridge.adminSetResource(eConfig.erc20Handler.address, destResourceId, eConfig.erc20.address); | |||
await eConfig.bridge.adminSetDecimals(eConfig.erc20Handler.address, eConfig.erc20.address, 18, 12, opts); | |||
// await eConfig.bridge.adminSetDecimals(eConfig.erc20Handler.address, eConfig.erc20.address, 18, 12, opts); |
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.
If this ain't needed, please remove it altogether :)
@@ -82,16 +82,24 @@ describeCrossChainTransfer('Test Cross-chain Transfer', ``, (context) => { | |||
const provider = context.ethConfig.wallets.alice.provider; | |||
const currentBlock = await provider.getBlockNumber(); | |||
await sleep(15); | |||
|
|||
let voteTransactionCount = 0; |
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.
What's this counter for? The loop stops after the first voteTransaction
anyway, so at the if (voteTransactionCount === 1) {
part voteTransactionCount
will always equal 1 🤔
if (decodedInput.name === 'voteProposal') { | ||
voteTransactionCount++; | ||
// We have threshold = 1 in this. So it really does not matter | ||
if (voteTransactionCount === 1) { |
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 intent can be made a bit more explicit by
if (voteTransactionCount === 1) { | |
if (voteTransactionCount === approvalThreshold) { |
with a const approvalThreshold = 1
earlier
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.
looks ok to me
This PR is not urgent and we will not change token bridge code in short time. |
This PR is for linking chainbridge ts test to latest binary.
In order to properly view this PR. Request code review in
Chainbridge Binary Change
litentry/ChainBridge#2
Chainbridge Solidity Change
litentry/chainbridge-solidity#1