From 1eb8a8b1dc727a38ffd5d3f4a7f260a683f523a4 Mon Sep 17 00:00:00 2001 From: Kirill Fedoseev Date: Thu, 6 Aug 2020 20:15:51 +0700 Subject: [PATCH 1/2] Use requestGasLimit parameter in AMB oracle in estimateGas (#415) --- oracle/src/confirmRelay.js | 9 ++++-- .../estimateGas.js | 11 ++++++- .../processAMBAffirmationRequests/index.js | 3 +- .../estimateGas.js | 8 ++++- .../processAMBCollectedSignatures/index.js | 3 +- oracle/src/sender.js | 9 ++++-- oracle/src/utils/constants.js | 4 +++ oracle/src/utils/message.js | 30 ++++++++++++++++++- oracle/src/utils/utils.js | 4 +-- oracle/test/message.test.js | 17 ++++++++++- oracle/test/utils.test.js | 8 +++++ 11 files changed, 94 insertions(+), 12 deletions(-) diff --git a/oracle/src/confirmRelay.js b/oracle/src/confirmRelay.js index 5b321d75..57f2c18a 100644 --- a/oracle/src/confirmRelay.js +++ b/oracle/src/confirmRelay.js @@ -7,7 +7,7 @@ const rpcUrlsManager = require('./services/getRpcUrlsManager') const { getNonce, getChainId, getEventsFromTx } = require('./tx/web3') const { sendTx } = require('./tx/sendTx') const { checkHTTPS, watchdog, syncForEach, addExtraGas } = require('./utils/utils') -const { EXIT_CODES, EXTRA_GAS_PERCENTAGE } = require('./utils/constants') +const { EXIT_CODES, EXTRA_GAS_PERCENTAGE, MAX_GAS_LIMIT } = require('./utils/constants') const { ORACLE_VALIDATOR_ADDRESS, ORACLE_VALIDATOR_ADDRESS_PRIVATE_KEY, ORACLE_ALLOW_HTTP_FOR_RPC } = process.env @@ -143,7 +143,12 @@ async function sendJobTx(jobs) { let nonce = await getNonce(web3Instance, ORACLE_VALIDATOR_ADDRESS) await syncForEach(jobs, async job => { - const gasLimit = addExtraGas(job.gasEstimate, EXTRA_GAS_PERCENTAGE) + let gasLimit + if (typeof job.extraGas === 'number') { + gasLimit = addExtraGas(job.gasEstimate + job.extraGas, 0, MAX_GAS_LIMIT) + } else { + gasLimit = addExtraGas(job.gasEstimate, EXTRA_GAS_PERCENTAGE, MAX_GAS_LIMIT) + } try { logger.info(`Sending transaction with nonce ${nonce}`) diff --git a/oracle/src/events/processAMBAffirmationRequests/estimateGas.js b/oracle/src/events/processAMBAffirmationRequests/estimateGas.js index 231b9326..c8ea4470 100644 --- a/oracle/src/events/processAMBAffirmationRequests/estimateGas.js +++ b/oracle/src/events/processAMBAffirmationRequests/estimateGas.js @@ -3,14 +3,23 @@ const { AlreadyProcessedError, AlreadySignedError, InvalidValidatorError } = req const logger = require('../../services/logger').child({ module: 'processAffirmationRequests:estimateGas' }) +const { parseAMBHeader } = require('../../utils/message') +const { strip0x } = require('../../../../commons') +const { + AMB_AFFIRMATION_REQUEST_EXTRA_GAS_ESTIMATOR: estimateExtraGas, + MIN_AMB_HEADER_LENGTH +} = require('../../utils/constants') async function estimateGas({ web3, homeBridge, validatorContract, message, address }) { try { const gasEstimate = await homeBridge.methods.executeAffirmation(message).estimateGas({ from: address }) + const msgGasLimit = parseAMBHeader(message).gasLimit + // message length in bytes + const len = strip0x(message).length / 2 - MIN_AMB_HEADER_LENGTH - return gasEstimate + return gasEstimate + msgGasLimit + estimateExtraGas(len) } catch (e) { if (e instanceof HttpListProviderError) { throw e diff --git a/oracle/src/events/processAMBAffirmationRequests/index.js b/oracle/src/events/processAMBAffirmationRequests/index.js index 19f7f0fd..cf0b61a6 100644 --- a/oracle/src/events/processAMBAffirmationRequests/index.js +++ b/oracle/src/events/processAMBAffirmationRequests/index.js @@ -4,7 +4,7 @@ const promiseLimit = require('promise-limit') const rootLogger = require('../../services/logger') const { web3Home } = require('../../services/web3') const bridgeValidatorsABI = require('../../../../contracts/build/contracts/BridgeValidators').abi -const { EXIT_CODES, MAX_CONCURRENT_EVENTS } = require('../../utils/constants') +const { EXIT_CODES, MAX_CONCURRENT_EVENTS, EXTRA_GAS_ABSOLUTE } = require('../../utils/constants') const estimateGas = require('./estimateGas') const { parseAMBMessage } = require('../../../../commons') const { AlreadyProcessedError, AlreadySignedError, InvalidValidatorError } = require('../../utils/errors') @@ -75,6 +75,7 @@ function processAffirmationRequestsBuilder(config) { txToSend.push({ data, gasEstimate, + extraGas: EXTRA_GAS_ABSOLUTE, transactionReference: affirmationRequest.transactionHash, to: config.homeBridgeAddress }) diff --git a/oracle/src/events/processAMBCollectedSignatures/estimateGas.js b/oracle/src/events/processAMBCollectedSignatures/estimateGas.js index 9ec943d1..f14134d6 100644 --- a/oracle/src/events/processAMBCollectedSignatures/estimateGas.js +++ b/oracle/src/events/processAMBCollectedSignatures/estimateGas.js @@ -4,6 +4,7 @@ const { AlreadyProcessedError, IncompatibleContractError, InvalidValidatorError const logger = require('../../services/logger').child({ module: 'processCollectedSignatures:estimateGas' }) +const { parseAMBHeader } = require('../../utils/message') const web3 = new Web3() const { toBN } = Web3.utils @@ -24,7 +25,12 @@ async function estimateGas({ const gasEstimate = await foreignBridge.methods.executeSignatures(message, signatures).estimateGas({ from: address }) - return gasEstimate + const msgGasLimit = parseAMBHeader(message).gasLimit + + // + estimateExtraGas(len) + // is not needed here, since estimateGas will already take into account gas + // needed for memory expansion, message processing, etc. + return gasEstimate + msgGasLimit } catch (e) { if (e instanceof HttpListProviderError) { throw e diff --git a/oracle/src/events/processAMBCollectedSignatures/index.js b/oracle/src/events/processAMBCollectedSignatures/index.js index 6f66ef5d..6985953f 100644 --- a/oracle/src/events/processAMBCollectedSignatures/index.js +++ b/oracle/src/events/processAMBCollectedSignatures/index.js @@ -8,7 +8,7 @@ const { signatureToVRS, packSignatures } = require('../../utils/message') const { parseAMBMessage } = require('../../../../commons') const estimateGas = require('./estimateGas') const { AlreadyProcessedError, IncompatibleContractError, InvalidValidatorError } = require('../../utils/errors') -const { MAX_CONCURRENT_EVENTS } = require('../../utils/constants') +const { MAX_CONCURRENT_EVENTS, EXTRA_GAS_ABSOLUTE } = require('../../utils/constants') const limit = promiseLimit(MAX_CONCURRENT_EVENTS) @@ -107,6 +107,7 @@ function processCollectedSignaturesBuilder(config) { txToSend.push({ data, gasEstimate, + extraGas: EXTRA_GAS_ABSOLUTE, transactionReference: colSignature.transactionHash, to: config.foreignBridgeAddress }) diff --git a/oracle/src/sender.js b/oracle/src/sender.js index 05025533..10515cb9 100644 --- a/oracle/src/sender.js +++ b/oracle/src/sender.js @@ -16,7 +16,7 @@ const { watchdog, nonceError } = require('./utils/utils') -const { EXIT_CODES, EXTRA_GAS_PERCENTAGE } = require('./utils/constants') +const { EXIT_CODES, EXTRA_GAS_PERCENTAGE, MAX_GAS_LIMIT } = require('./utils/constants') const { ORACLE_VALIDATOR_ADDRESS_PRIVATE_KEY } = process.env @@ -106,7 +106,12 @@ async function main({ msg, ackMsg, nackMsg, channel, scheduleForRetry }) { logger.debug(`Sending ${txArray.length} transactions`) await syncForEach(txArray, async job => { - const gasLimit = addExtraGas(job.gasEstimate, EXTRA_GAS_PERCENTAGE) + let gasLimit + if (typeof job.extraGas === 'number') { + gasLimit = addExtraGas(job.gasEstimate + job.extraGas, 0, MAX_GAS_LIMIT) + } else { + gasLimit = addExtraGas(job.gasEstimate, EXTRA_GAS_PERCENTAGE, MAX_GAS_LIMIT) + } try { logger.info(`Sending transaction with nonce ${nonce}`) diff --git a/oracle/src/utils/constants.js b/oracle/src/utils/constants.js index 5641f37e..7d6fc24c 100644 --- a/oracle/src/utils/constants.js +++ b/oracle/src/utils/constants.js @@ -1,5 +1,9 @@ module.exports = { EXTRA_GAS_PERCENTAGE: 4, + EXTRA_GAS_ABSOLUTE: 200000, + AMB_AFFIRMATION_REQUEST_EXTRA_GAS_ESTIMATOR: len => Math.floor(0.0035 * len ** 2 + 40 * len), + MIN_AMB_HEADER_LENGTH: 32 + 20 + 20 + 4 + 2 + 1 + 2, + MAX_GAS_LIMIT: 10000000, MAX_CONCURRENT_EVENTS: 50, RETRY_CONFIG: { retries: 20, diff --git a/oracle/src/utils/message.js b/oracle/src/utils/message.js index 894a4775..5be3ae3b 100644 --- a/oracle/src/utils/message.js +++ b/oracle/src/utils/message.js @@ -73,9 +73,37 @@ function packSignatures(array) { return `0x${msgLength}${v}${r}${s}` } +function parseAMBHeader(message) { + message = strip0x(message) + + const messageIdStart = 0 + const messageIdLength = 32 * 2 + const messageId = `0x${message.slice(messageIdStart, messageIdStart + messageIdLength)}` + + const senderStart = messageIdStart + messageIdLength + const senderLength = 20 * 2 + const sender = `0x${message.slice(senderStart, senderStart + senderLength)}` + + const executorStart = senderStart + senderLength + const executorLength = 20 * 2 + const executor = `0x${message.slice(executorStart, executorStart + executorLength)}` + + const gasLimitStart = executorStart + executorLength + const gasLimitLength = 4 * 2 + const gasLimit = parseInt(message.slice(gasLimitStart, gasLimitStart + gasLimitLength), 16) + + return { + messageId, + sender, + executor, + gasLimit + } +} + module.exports = { createMessage, parseMessage, signatureToVRS, - packSignatures + packSignatures, + parseAMBHeader } diff --git a/oracle/src/utils/utils.js b/oracle/src/utils/utils.js index eb3a197e..a191e2cb 100644 --- a/oracle/src/utils/utils.js +++ b/oracle/src/utils/utils.js @@ -48,13 +48,13 @@ async function waitForFunds(web3, address, minimumBalance, cb, logger) { ) } -function addExtraGas(gas, extraPercentage) { +function addExtraGas(gas, extraPercentage, maxGasLimit = Infinity) { gas = BigNumber(gas) extraPercentage = BigNumber(1 + extraPercentage) const gasWithExtra = gas.multipliedBy(extraPercentage).toFixed(0) - return BigNumber(gasWithExtra) + return BigNumber.min(maxGasLimit, gasWithExtra) } function setIntervalAndRun(f, interval) { diff --git a/oracle/test/message.test.js b/oracle/test/message.test.js index dce8abe0..07cee88e 100644 --- a/oracle/test/message.test.js +++ b/oracle/test/message.test.js @@ -1,6 +1,6 @@ const { BN, toBN } = require('web3').utils const { expect } = require('chai').use(require('bn-chai')(BN)) -const { createMessage, parseMessage, signatureToVRS } = require('../src/utils/message') +const { createMessage, parseMessage, signatureToVRS, parseAMBHeader } = require('../src/utils/message') describe('message utils', () => { const expectedMessageLength = 104 @@ -288,4 +288,19 @@ describe('message utils', () => { expect(signatureThunk).to.throw() }) }) + describe('parseAMBHeader', () => { + it('should return correct values for parsed headers', () => { + // given + const message = + '0x000500009a6ff99b356dd998260582be7d95a4d08b2132600000000000000061339d0e6f308a410f18888932bdf661636a0f538f34718200957aeadd6bece186e61b95618e73a6dc000f42400101002a4d2fb102cf00000000000000000000000081c770bbe8f5f41b4642ed575e630c911c94e4070000000000000000000000000000000000000000000000000000000000000002000000000000000000000000000000000000000000000000000000000000008000000000000000000000000000000000000000000000000000000000000000e0000000000000000000000000000000000000000000000000000000000000002e516d543162324178466b6643456a6f715861547148734370666f724a4c66765667434853516e513847575a347662000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000002e516d5a7543586f693378487338486e4c325a423539316674466f4c454471516b473655746e47324d6d513147614e000000000000000000000000000000000000' + + const { messageId, sender, executor, gasLimit } = parseAMBHeader(message) + + // then + expect(messageId).to.equal('0x000500009a6ff99b356dd998260582be7d95a4d08b2132600000000000000061') + expect(sender).to.equal('0x339d0e6f308a410f18888932bdf661636a0f538f') + expect(executor).to.equal('0x34718200957aeadd6bece186e61b95618e73a6dc') + expect(gasLimit).to.equal(1000000) + }) + }) }) diff --git a/oracle/test/utils.test.js b/oracle/test/utils.test.js index 494e6dca..231ab3e0 100644 --- a/oracle/test/utils.test.js +++ b/oracle/test/utils.test.js @@ -34,6 +34,14 @@ describe('utils', () => { expect(result.toString()).to.equal('225') }) + + it('should handle maxGasLimit', () => { + const result1 = addExtraGas(new BigNumber(100), 0.25, 110) + const result2 = addExtraGas(new BigNumber(100), 0.25, 150) + + expect(result1.toString()).to.equal('110') + expect(result2.toString()).to.equal('125') + }) }) describe('checkHTTPS', () => { From f3f226afdff656d108e5db20f9ddbb0ab8a39571 Mon Sep 17 00:00:00 2001 From: Alexander Kolotov Date: Thu, 6 Aug 2020 23:43:13 +0300 Subject: [PATCH 2/2] Correction of the message for a transaction without the bridge requests (#416) --- alm/src/components/StatusContainer.tsx | 23 +++++++++++++++++------ alm/src/config/descriptions.ts | 3 ++- 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/alm/src/components/StatusContainer.tsx b/alm/src/components/StatusContainer.tsx index bd96d778..c5d9e9ef 100644 --- a/alm/src/components/StatusContainer.tsx +++ b/alm/src/components/StatusContainer.tsx @@ -64,10 +64,6 @@ export const StatusContainer = ({ onBackToMain, setNetworkFromParams, receiptPar const displayReference = multiMessageSelected ? messages[selectedMessageId].id : txHash const formattedMessageId = formatTxHash(displayReference) - const displayedDescription = multiMessageSelected - ? getTransactionStatusDescription(TRANSACTION_STATUS.SUCCESS_ONE_MESSAGE, timestamp) - : description - const isHome = chainId === home.chainId.toString() const txExplorerLink = getExplorerTxUrl(txHash, isHome) const displayExplorerLink = status !== TRANSACTION_STATUS.NOT_FOUND @@ -75,17 +71,32 @@ export const StatusContainer = ({ onBackToMain, setNetworkFromParams, receiptPar const displayConfirmations = status === TRANSACTION_STATUS.SUCCESS_ONE_MESSAGE || multiMessageSelected const messageToConfirm = messages.length > 1 ? messages[selectedMessageId] : messages.length > 0 ? messages[0] : { id: '', data: '' } + + let displayedDescription: string = multiMessageSelected + ? getTransactionStatusDescription(TRANSACTION_STATUS.SUCCESS_ONE_MESSAGE, timestamp) + : description + let link + const descArray = displayedDescription.split('%link') + if (descArray.length > 1) { + displayedDescription = descArray[0] + link = ( + + {descArray[1]} + + ) + } + return (
{status && (

- The request{' '} + The transaction{' '} {displayExplorerLink && ( {formattedMessageId} )} - {!displayExplorerLink && } {displayedDescription} + {!displayExplorerLink && } {displayedDescription} {link}

)} {displayMessageSelector && } diff --git a/alm/src/config/descriptions.ts b/alm/src/config/descriptions.ts index d030db50..dd2ed174 100644 --- a/alm/src/config/descriptions.ts +++ b/alm/src/config/descriptions.ts @@ -2,7 +2,8 @@ export const TRANSACTION_STATUS_DESCRIPTION: { [key: string]: string } = { SUCCESS_MULTIPLE_MESSAGES: 'was initiated %t and contains several bridge messages. Specify one of them:', SUCCESS_ONE_MESSAGE: 'was initiated %t', - SUCCESS_NO_MESSAGES: 'execution succeeded %t but it does not contain any bridge messages', + SUCCESS_NO_MESSAGES: + 'successfully mined %t but it does not seem to contain any request to the bridge, \nso nothing needs to be confirmed by the validators. \nIf you are sure that the transaction should contain a request to the bridge,\ncontact to the validators by \nmessaging on %linkhttps://forum.poa.network/c/support', FAILED: 'failed %t', NOT_FOUND: 'Transaction not found. \n1. Check that the transaction hash is correct. \n2. Wait several blocks for the transaction to be\nmined, gas price affects mining speed.'