Add support for AMB contracts #199

patitonar · 2019-09-06T11:59:11Z

This PR includes:

Oracle support for AMB
Monitor support for AMB
Oracle-e2e updated with AMB tests
Contract submodule updated to a commit that includes AMB work. Once the set of contracts is released, we should update the submodule to the new version.

monitor-e2e should be updated to include tests for AMB. We can create an issue to later address this.

# Conflicts: # oracle-e2e/run-tests.sh

# Conflicts: # oracle-e2e/deploy.js # oracle-e2e/docker-compose.yml # oracle-e2e/run-tests.sh # oracle/config/base.config.js # oracle/src/services/gasPrice.js # oracle/src/utils/utils.js # oracle/test/gasPrice.test.js

# Conflicts: # .prettierrc # commons/abis.js # commons/package.json # e2e-commons/docker-compose.yml # monitor/eventsStats.js # monitor/utils/events.js # oracle/src/services/gasPrice.js # oracle/test/gasPrice.test.js # yarn.lock

rzadp

Looks like it's missing explanation in Operational Modes and in Oracle Architecture

rzadp · 2019-09-09T09:29:12Z

oracle/src/events/processAMBCollectedSignatures/index.js

+            gasPriceOptions
+          })
+        } else {
+          logger.info(`Validator not responsible for relaying CollectedSignatures ${colSignature.transactionHash}`)


It might be worth to change it to guard clause in order to reduce nesting, i.e.:

if (authorityResponsibleForRelay !== web3Home.utils.toChecksumAddress(config.validatorAddress)) { logger.info(`Validator not responsible for relaying CollectedSignatures ${colSignature.transactionHash}`) return } logger.info(`Processing CollectedSignatures ${colSignature.transactionHash}`) (...)

Applicable to existing processCollectedSignatures/index.js as well.

Thanks! Please check 0aa3b20

rzadp · 2019-09-09T09:33:59Z

oracle/src/events/processAMBCollectedSignatures/index.js

+
+    rootLogger.debug(`Processing ${signatures.length} CollectedSignatures events`)
+    const callbacks = signatures.map(colSignature =>
+      limit(async () => {


It might be worth to replace it in order to reduce nesting:

const callbacks = signatures.map(colSignature => async () => { const { authorityResponsibleForRelay, messageHash, NumberOfCollectedSignatures } = colSignature.returnValues (...) }.map(promise => limit(promise))

Applicable to other existing usages of promise-limit as well

Thanks! Applied the changes, please take a look da69c64

rzadp · 2019-09-09T10:01:18Z

oracle/src/events/processAMBCollectedSignatures/index.js

+
+          const requiredSignatures = []
+          requiredSignatures.length = NumberOfCollectedSignatures
+          requiredSignatures.fill(0)


Could be replaced with:

const requiredSignatures = (new Array(NumberOfCollectedSignatures)).fill(0)

Fixed 20bbd3a

akolotov

@patitonar could you simplify the delta by removing everything that it is related to the defrayal mode that was postponed?

# Conflicts: # contracts

patitonar · 2019-09-11T15:25:23Z

@akolotov Removed everything related to defrayal mode

akolotov

Please merge accurately with the master - there are lots of changes related to configuration parameters.

akolotov · 2019-09-11T17:23:47Z

monitor/getBalances.js

@@ -107,6 +107,22 @@ async function main(bridgeMode) {
      balanceDiff: Number(Web3Utils.fromWei(diff)),
      lastChecked: Math.floor(Date.now() / 1000)
    }
+  } else if (bridgeMode === BRIDGE_MODES.ARBITRARY_MESSAGE) {


don't think that it make sense to measure balance difference for pure AMB. It will be required for the token bridges on top of AMB.
For AMB let's return {home:{},foreign:{},lastChecked:xxxxx}

Done ef447b7

akolotov · 2019-09-17T19:06:44Z

deployment/roles/oracle/templates/.env.j2

@@ -9,7 +9,9 @@ HOME_POLLING_INTERVAL={{ HOME_POLLING_INTERVAL }}
 ## Foreign contract
 FOREIGN_RPC_URL={{ FOREIGN_RPC_URL }}
 FOREIGN_BRIDGE_ADDRESS={{ FOREIGN_BRIDGE_ADDRESS }}
+{% if ERC20_TOKEN_ADDRESS | default('') != '' %}


it should disappear after merge

Yes, it disappeared

akolotov · 2019-09-17T19:15:45Z

monitor/alerts.js

+  let xSignatures
+  let xAffirmations
+  if (bridgeMode === BRIDGE_MODES.ARBITRARY_MESSAGE) {
+    xSignatures = foreignDeposits.filter(processedMsgNotDelivered(homeDeposits))


please consider either to change names foreignDeposits, homeDeposits, homeWithdrawals and foreignWithdrawals to more common (e.g. foreignToHomeRequests, homeToForeignRequests, foreignToHomeConfirmations and homeToForeignConfirmations) under this PR or create another issue to address it later.

Thanks for suggestion, fixed 4370fd2

akolotov · 2019-09-17T19:21:24Z

monitor/eventsStats.js

@@ -57,25 +58,45 @@ function compareTransferForeign(home) {
 }

 async function main() {
-  const { foreignDeposits, homeDeposits, homeWithdrawals, foreignWithdrawals, isExternalErc20 } = await eventsInfo()
+  const {
+    foreignDeposits,


please see comment for changes in alert.js

akolotov · 2019-09-17T19:24:10Z

monitor/getShortEventStats.js

@@ -1,19 +1,35 @@
 require('dotenv').config()
 const eventsInfo = require('./utils/events')
+const { BRIDGE_MODES } = require('../commons')

 async function main(bridgeMode) {
  const { foreignDeposits, homeDeposits, homeWithdrawals, foreignWithdrawals } = await eventsInfo(bridgeMode)


please see comment for changes in alert.js

akolotov · 2019-09-17T19:33:21Z

monitor/getShortEventStats.js

-      withdrawals: foreignWithdrawals.length
+  if (bridgeMode === BRIDGE_MODES.ARBITRARY_MESSAGE) {
+    return {
+      deliveryDiff: homeDeposits.length - foreignDeposits.length,


for AMB it is necessary to change checkWorkers.js as so it outputs:

{"home":{"toForeign":1513,"fromForeign":1651},"foreign":{"fromHome":1513,"toHome":1655}, "lastChecked":1568748063,"fromHomeToForeignDiff":0,"fromForeignToHomeDiff":-4,"timeDiff":69}

Updated the output. Please check 9c87c23

# Conflicts: # deployment/roles/oracle/templates/.env.j2 # monitor/utils/events.js

patitonar added 30 commits May 24, 2019 15:42

add amb base implementation on oracle

7ca3b00

use gas price from message on gas estimate for amb events

95f4a0a

Update submodule to amb branch

0326745

add amb oracle e2e tests

e006443

Merge branch 'master' into amb-oracle

a260370

# Conflicts: # oracle-e2e/run-tests.sh

Use compiled amb contracts on oracle

80940a0

Merge branch 'master' into amb-oracle

0694fc2

# Conflicts: # oracle-e2e/deploy.js # oracle-e2e/docker-compose.yml # oracle-e2e/run-tests.sh # oracle/config/base.config.js # oracle/src/services/gasPrice.js # oracle/src/utils/utils.js # oracle/test/gasPrice.test.js

Oracle amb merge fix

70d03c1

Add ARBITRARY_MESSAGE bridge mode

478b30b

Lint fix

ac6775b

Fix e2e-commons up script

8cf97ae

Fix commons unit test

473b6a3

Update contract submodule

aa32f2c

Update contract module name

e8a179b

Update amb contract submodule

a056a75

Update amb contract submodule

35c2e64

Update submodule to latest amb contract changes

48620a8

Update deployment oracle template

08d2c46

Support amb balances info in monitor

3d0f68f

Support amb events info in monitor

751b885

Move amb message utils to commons

e5ce665

Update contract submodule

55f8443

Fix amb events in monitor

ee4a6c8

Fix prettier

2d42c23

Fix amb monitor

4b59f18

Update contract submodule

e1f20b8

Update amb message processing

383e22c

Update submodule

ab5166b

Pack signatures when processing amb collectedSignatures event in Oracle

43609d7

Merge branch 'master' into amb-oracle

d22a3e8

# Conflicts: # .prettierrc # commons/abis.js # commons/package.json # e2e-commons/docker-compose.yml # monitor/eventsStats.js # monitor/utils/events.js # oracle/src/services/gasPrice.js # oracle/test/gasPrice.test.js # yarn.lock

Fix amb tests in oracle-e2e

4453e8a

patitonar added oracle related to TokenBridge oracle monitor related to TokenBridge monitor labels Sep 6, 2019

patitonar requested a review from akolotov September 6, 2019 11:59

patitonar self-assigned this Sep 6, 2019

akolotov requested a review from rzadp September 6, 2019 12:11

rzadp reviewed Sep 9, 2019

View reviewed changes

akolotov requested changes Sep 11, 2019

View reviewed changes

patitonar added 5 commits September 11, 2019 10:42

Merge branch 'master' into amb-oracle

88de919

# Conflicts: # contracts

Remove logic related to amb defrayal mode

9b50bc4

Refactor required signatures array initialization in Oracle

20bbd3a

Change to guard close in authority responsible for relay check

0aa3b20

Update promise-limit usage

da69c64

akolotov requested changes Sep 17, 2019

View reviewed changes

akolotov changed the base branch from master to develop September 17, 2019 19:50

patitonar added 8 commits September 18, 2019 09:28

Merge branch 'develop' into amb-oracle

40d227f

# Conflicts: # deployment/roles/oracle/templates/.env.j2 # monitor/utils/events.js

Fix merge e2e envs

de7dbd5

Merge fix monitor

f43439b

Fix oracle amb signatureRequests

f06de37

Remove balances information for AMB contracts in monitor

ef447b7

Update information displayed for AMB contracts in monitor

9c87c23

Improve events vars naming in monitor

4370fd2

Add amb information to docs

abb59e1

akolotov approved these changes Sep 18, 2019

View reviewed changes

akolotov merged commit d577a71 into develop Sep 18, 2019

akolotov deleted the amb-oracle branch September 18, 2019 19:45

akolotov mentioned this pull request Oct 18, 2019

Merge the develop branch to the master branch, preparation to v1.0.0-rc2 #220

Merged

akolotov added a commit that referenced this pull request Oct 21, 2019

Add support for AMB contracts (#199)

9884b4b

akolotov mentioned this pull request Nov 18, 2019

Changes in initialization of array to iterate getting signatures #240

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for AMB contracts #199

Add support for AMB contracts #199

patitonar commented Sep 6, 2019

rzadp left a comment •

edited

Loading

rzadp Sep 9, 2019

patitonar Sep 11, 2019

rzadp Sep 9, 2019

patitonar Sep 11, 2019

rzadp Sep 9, 2019

patitonar Sep 11, 2019

akolotov left a comment

patitonar commented Sep 11, 2019

akolotov left a comment

akolotov Sep 11, 2019

patitonar Sep 18, 2019

akolotov Sep 17, 2019

patitonar Sep 18, 2019

akolotov Sep 17, 2019

patitonar Sep 18, 2019

akolotov Sep 17, 2019

akolotov Sep 17, 2019

akolotov Sep 17, 2019

patitonar Sep 18, 2019

Add support for AMB contracts #199

Add support for AMB contracts #199

Conversation

patitonar commented Sep 6, 2019

rzadp left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

akolotov left a comment

Choose a reason for hiding this comment

patitonar commented Sep 11, 2019

akolotov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rzadp left a comment •

edited

Loading