Merge pull request #2562 from ethereum/receiveF

Receive bug and more tests
5 years ago · 5de17f6f0a
parent ae1d1c2e08 ff2c5eac9b
commit 5de17f6f0a
3 changed files with 104 additions and 30 deletions
--- a/src/app/ui/universal-dapp-ui.js
+++ b/src/app/ui/universal-dapp-ui.js
@ -133,7 +133,7 @@ UniversalDAppUI.prototype.renderInstanceFromABI = function (contractABI, address
        <label class="pt-2 border-top d-flex justify-content-start flex-grow-1">
          Low level interactions with contract
        </label>
-        <a href="https://solidity.readthedocs.io/en/v0.6.2/contracts.html#receive-ether-function" class="" title="the link to documentation" target="_blank">
+        <a href="https://solidity.readthedocs.io/en/v0.6.2/contracts.html#receive-ether-function" title="the link to documentation" target="_blank">
          <i aria-hidden="true" class="fas fa-info text-info my-2 mr-2"></i>
        </a>
      </div>
@ -151,42 +151,55 @@ UniversalDAppUI.prototype.renderInstanceFromABI = function (contractABI, address
  `

  function sendData () {
-    let error = false
    function setLLIError (text) {
      llIError.innerText = text
-      if (text !== '') error = true
    }

    setLLIError('')
    const fallback = self.udapp.getFallbackInterface(contractABI)
    const receive = self.udapp.getReceiveInterface(contractABI)
    const args = {
-      funABI: fallback,
+      funABI: fallback || receive,
      address: address,
      contractName: contractName,
      contractABI: contractABI
    }
-    let calldata = calldataInput.value
    const amount = document.querySelector('#value').value
    if (amount !== '0') {
      // check for numeric and receive/fallback
      if (!helper.isNumeric(amount)) {
-        setLLIError('Value to send should be a number')
+        return setLLIError('Value to send should be a number')
      } else if (!receive && !(fallback && fallback.stateMutability === 'payable')) {
-        setLLIError("In order to receive Ether transfer the contract should have either 'receive' or payable 'fallback' function")
+        return setLLIError("In order to receive Ether transfer the contract should have either 'receive' or payable 'fallback' function")
      }
    }
+    let calldata = calldataInput.value
    if (calldata) {
-      if (calldata.length > 3 && calldata.substr(0, 2) === '0x') {
-        if (!helper.isHexadecimal(calldata.substr(2, calldata.length))) {
-          setLLIError('the calldata should be a valid hexadecimal value.')
+      if (calldata.length < 2 || calldata.length < 4 && helper.is0XPrefixed(calldata)) {
+        return setLLIError('the calldata should be a valid hexadecimal value with size of at least one byte.')
+      } else {
+        if (helper.is0XPrefixed(calldata)) {
+          calldata = calldata.substr(2, calldata.length)
+        }
+        if (!helper.isHexadecimal(calldata)) {
+          return setLLIError('the calldata should be a valid hexadecimal value with size of at least one byte.')
        }
      }
      if (!fallback) {
-        setLLIError("'fallback' function is not defined")
+        return setLLIError("'Fallback' function is not defined")
      }
    }
-    if ((calldata || amount !== '0') && !error) self.runTransaction(false, args, null, calldata, null)
+
+    if (!receive && !fallback) return setLLIError(`Both 'receive' and 'fallback' functions are not defined`)
+
+    // we have to put the right function ABI:
+    // if receive is defined and that there is no calldata => receive function is called
+    // if fallback is defined => fallback function is called
+    if (receive && !calldata) args.funABI = receive
+    else if (fallback) args.funABI = fallback
+
+    if (!args.funABI) return setLLIError(`Please define a 'Fallback' function to send calldata and a either 'Receive' or payable 'Fallback' to send ethers`)
+    self.runTransaction(false, args, null, calldataInput.value, null)
  }

  contractActionsWrapper.appendChild(lowLevelInteracions)
--- a/src/lib/helper.js
+++ b/src/lib/helper.js
@ -45,7 +45,10 @@ module.exports = {
    return name.match(/[:*?"<>\\'|]/) != null
  },
  isHexadecimal (value) {
-    return /^[0-9a-fA-F]+$/.test(value)
+    return /^[0-9a-fA-F]+$/.test(value) && (value.length % 2 === 0)
+  },
+  is0XPrefixed (value) {
+    return value.substr(0, 2) === '0x'
  },
  isNumeric (value) {
    return /^\+?(0|[1-9]\d*)$/.test(value)
--- a/test-browser/tests/specialFunctions.js
+++ b/test-browser/tests/specialFunctions.js
@ -2,15 +2,6 @@
 var init = require('../helpers/init')
 var sauce = require('./sauce')

-/**
- * both are declared, sending data
- * both are declared - receive called, sending wei
- * both are declared - fallback should fail cause not payable, sending data and wei
- * receive is declared, failing, fallback is not declared, sending data
- * receive is not declared, fallback is payable, sending wei
- * receive is not declared, fallback is payable, sending data and wei
- * both are not declared, sending data and wei, should fail
- */
 module.exports = {
  before: function (browser, done) {
    init(browser, done)
@ -36,7 +27,19 @@ module.exports = {
      })
    })
  },
-  'Use special functions receive/follback - both are declared - receive called, sending wei': function (browser) {
+  'Use special functions receive/fallback - both are declared, failing sending data < 1 byte': function (browser) {
+    // don't need to redeploy it, same contract
+    browser.perform((done) => {
+      browser.getAddressAtPosition(0, (address) => {
+        browser.sendLowLevelTx(address, '0', '0xa')
+        .pause(1000)
+        .waitForElementVisible(`#instance${address} label[id="deployAndRunLLTxError"]`)
+        .assert.containsText(`#instance${address} label[id="deployAndRunLLTxError"]`, `the calldata should be a valid hexadecimal value with size of at least one byte.`)
+        .perform(done)
+      })
+    })
+  },
+  'Use special functions receive/fallback - both are declared - receive called, sending wei': function (browser) {
    // don't need to redeploy it, same contract
    browser.perform((done) => {
      browser.getAddressAtPosition(0, (address) => {
@ -49,7 +52,7 @@ module.exports = {
      })
    })
  },
-  'Use special functions receive/follback - both are declared - fallback should fail cause not payable, sending data and wei': function (browser) {
+  'Use special functions receive/fallback - both are declared - fallback should fail cause not payable, sending data and wei': function (browser) {
    // don't need to redeploy it, same contract
    browser.perform((done) => {
      browser.getAddressAtPosition(0, (address) => {
@ -61,7 +64,7 @@ module.exports = {
      })
    })
  },
-  'Use special functions receive/follback - receive is declared, failing, fallback is not declared, sending data': function (browser) {
+  'Use special functions receive/fallback - only receive is declared, sending wei': function (browser) {
    browser.waitForElementVisible('#icon-panel', 10000)
    .testContracts('receiveOnly.sol', sources[1]['browser/receiveOnly.sol'], ['CheckSpecials'])
    .clickLaunchIcon('udapp')
@ -69,16 +72,29 @@ module.exports = {
    .createContract('')
    .clickInstance(1)
    .perform((done) => {
+      browser.getAddressAtPosition(1, (address) => {
+        browser.sendLowLevelTx(address, '1', '')
+        .pause(1000)
+        .journalLastChildIncludes('to:CheckSpecials.(receive)')
+        .journalLastChildIncludes('value:1 wei')
+        .journalLastChildIncludes('data:0x')
+        .perform(done)
+      })
+    })
+  },
+  'Use special functions receive/fallback - only receive is declared, failing, fallback is not declared, sending data': function (browser) {
+    // don't need to redeploy it, same contract
+    browser.perform((done) => {
      browser.getAddressAtPosition(1, (address) => {
        browser.sendLowLevelTx(address, '0', '0xaa')
        .pause(1000)
        .waitForElementVisible(`#instance${address} label[id="deployAndRunLLTxError"]`)
-        .assert.containsText(`#instance${address} label[id="deployAndRunLLTxError"]`, `'fallback' function is not defined`)
+        .assert.containsText(`#instance${address} label[id="deployAndRunLLTxError"]`, `'Fallback' function is not defined`)
        .perform(done)
      })
    })
  },
-  'Use special functions receive/fallback - receive is not declared, fallback is payable, sending wei': function (browser) {
+  'Use special functions receive/fallback - only fallback declared and is payable, sending wei': function (browser) {
    browser.waitForElementVisible('#icon-panel', 10000)
    .testContracts('fallbackOnlyPayable.sol', sources[2]['browser/fallbackOnlyPayable.sol'], ['CheckSpecials'])
    .clickLaunchIcon('udapp')
@ -96,7 +112,7 @@ module.exports = {
      })
    })
  },
-  'Use special functions receive/follback - receive is not declared, fallback is payable, sending data and wei': function (browser) {
+  'Use special functions receive/fallback - only fallback is diclared and is payable, sending data and wei': function (browser) {
    // don't need to redeploy it, same contract
    browser.perform((done) => {
      browser.getAddressAtPosition(2, (address) => {
@ -109,7 +125,7 @@ module.exports = {
      })
    })
  },
-  'Use special functions receive/fallback - receive is not declared, fallback should fail cause not payable, sending wei': function (browser) {
+  'Use special functions receive/fallback - only fallback is declared, fallback should fail cause not payable, sending wei': function (browser) {
    browser.waitForElementVisible('#icon-panel', 10000)
    .testContracts('fallbackOnlyNotPayable.sol', sources[3]['browser/fallbackOnlyNotPayable.sol'], ['CheckSpecials'])
    .clickLaunchIcon('udapp')
@ -136,7 +152,7 @@ module.exports = {
    .setValue('#value', 0)
    .createContract('')
    .clickInstance(4)
-    .pause(10000)
+    .pause(1000)
    .perform((done) => {
      browser.getAddressAtPosition(4, (address) => {
        browser.sendLowLevelTx(address, '1', '0xaa')
@ -147,6 +163,39 @@ module.exports = {
        .perform(done)
      })
    })
+  },
+  'Use special functions receive/fallback - receive and fallback are declared and payable, sending wei': function (browser) {
+    browser.perform((done) => {
+      browser.getAddressAtPosition(4, (address) => {
+        browser.sendLowLevelTx(address, '1', '')
+        .pause(1000)
+        .journalLastChildIncludes('to:CheckSpecials.(receive)')
+        .journalLastChildIncludes('value:1 wei')
+        .journalLastChildIncludes('data:0x')
+        .perform(done)
+      })
+    })
+  },
+  'Use special functions receive/fallback - receive and fallback are not declared, sending nothing': function (browser) {
+    browser.waitForElementVisible('#icon-panel', 10000)
+    .testContracts('notSpecial.sol', sources[5]['browser/notSpecial.sol'], ['CheckSpecials'])
+    .clickLaunchIcon('udapp')
+    .selectContract('CheckSpecials')
+    .waitForElementVisible('#value')
+    .clearValue('#value')
+    .setValue('#value', 0)
+    .createContract('')
+    .clickInstance(5)
+    .pause(1000)
+    .perform((done) => {
+      browser.getAddressAtPosition(5, (address) => {
+        browser.sendLowLevelTx(address, '0', '')
+        .pause(1000)
+        .waitForElementVisible(`#instance${address} label[id="deployAndRunLLTxError"]`)
+        .assert.containsText(`#instance${address} label[id="deployAndRunLLTxError"]`, `Both 'receive' and 'fallback' functions are not defined`)
+        .perform(done)
+      })
+    })
    .end()
  },
  tearDown: sauce
@ -199,5 +248,14 @@ var sources = [
        }
      `
    }
+  },
+  {
+    'browser/notSpecial.sol': {
+      content: `
+        contract CheckSpecials {
+          function otherFallback() payable external {}
+        }
+      `
+    }
  }
 ]