Fix off-by-one error in decodeLog

[wbt]

topics[0] identifies the event; the first indexed param is topics[1].

Description

When trying to use web3.eth.abi.decodeLog on an event with two indexed parameters (an int40 and uint32), both of which were fairly small integers with the first constant between tests and the second increasing by steps of the first, I was regularly getting a constant large negative number for the first and a constant small number (matching the value of the first) for the second. This is because topics[0] is used to identify which event a log is for, and the indexed parameters follow.

Collaboration for the unchecked items below is welcome.

Type of change

• Bug fix (non-breaking change which fixes an issue)

Checklist:

• I have selected the correct base branch.
• I have performed a self-review of my own code.
• I have commented my code, particularly in hard-to-understand areas.
• I have made corresponding changes to the documentation.
• My changes generate no new warnings.
• Any dependent changes have been merged and published in downstream modules.
• I ran npm run dtslint with success and extended the tests and types if necessary.
• I ran npm run test:unit with success.
• I ran npm run test:cov and my test cases cover all the lines and branches of the added code.
• I ran npm run build and tested dist/web3.min.js in a browser.
• I have tested my code on the live network.
• I have checked the Deploy Preview and it looks correct.
• I have updated the CHANGELOG.md file in the root folder.

[GregTheGreek]

Thanks again @wbt could you provide examples of the errors you were seeing. Would help greatly to approve this :)

[wbt]

Here is a simplified example set of reproduction steps:

In shell with truffle globally installed:

mkdir aggregator
cd aggregator
truffle init
npm init (accept all defaults is fine)
npm i web3 (optional)
truffle version

Truffle v5.1.35 (core: 5.1.35)
Solidity - ^0.6.5 (solc-js)
Node v9.3.0
Web3.js v1.2.1

New file contracts/Aggregator.sol:

//SPDX-License-Identifier: GPL
pragma solidity ^0.6.5;
contract Aggregator {
    uint32 public currentTotal;

    //The first parameter is SIGNED and negative in functions removed from
    //this simplified example; it is larger to accommodate the sign bit.
    event TotalChanged(int40 indexed howMuchAdded, uint32 indexed newTotal);

    function addToTotal(uint32 howMuch) public returns (uint32 newTotal) {
        emit TotalChanged(howMuch, (newTotal = currentTotal+howMuch));
        currentTotal = newTotal;
    }
}

New file migrations/2_contract_deploy.js:

var Aggregator = artifacts.require("Aggregator");

module.exports = function(deployer, network, accounts) {
    deployer.deploy(Aggregator);
};

New file migrations/3_aggregate.js:

var Aggregator = artifacts.require("Aggregator");
var Web3 = require('web3');
const wait = ms => new Promise((r, j)=>setTimeout(r, ms));
const blockTime = 5000; //ms; change this to your network's block time

module.exports = function(deployer, network, accounts) {
    let aggregatorInstance, txHash;
    let web3Local = new Web3(deployer.provider);
    console.log("Using web3 version",web3Local.version);
    deployer.then(function() {
        return Aggregator.deployed();
    }).then(function(instance) {
        aggregatorInstance = instance;
        return startTransaction(aggregatorInstance);
    }).then(function(hash) {
        txHash = hash;
        return wait(blockTime*2); //needed only with geth, not with automining ganache
    }).then(function() {
        return web3Local.eth.getTransactionReceipt(txHash);
    }).then(function(receipt) {
        if(receipt == null || !receipt.status) {
            console.log("Receipt does not reflect completed transaction:",receipt);
        } else {
            console.log("Receipt: ",receipt);
            let log = receipt.logs[0];
            let logABI = aggregatorInstance.constructor.events[log.topics[0]];
            let parsedLogs = web3Local.eth.abi.decodeLog(logABI.inputs, log.data, log.topics);
            console.log("Calling web3.eth.abi.decodeLog(",logABI.inputs,",",
            log.data,",", log.topics,") produced ",parsedLogs);
            console.log("Added "+parsedLogs['howMuchAdded']+
            " to the aggregator; the new total is "+parsedLogs['newTotal']+".");
        }
    }).catch(function(err) {
        console.log(err.message);
    });
};

startTransaction = function(aggregatorInstance) {
    return new Promise(function(resolve, reject) {
        aggregatorInstance.contract.methods.addToTotal(16).send(
            {from: Aggregator.defaults().from},
            function(error, data) {
                if(error) {
                    reject(error);
                } else {
                    if(typeof data === 'object') {
                        reject("Object returned from method call instead of transaction hash.");
                    } else {
                        resolve(data);
                    }
                }
            }
        )
    });
}

In truffle-config.js:

  compilers: {
      solc: {
          version: "^0.6.5"
      }
  },

Also have Ganache running and/or set up network connections in truffle-config, having the node running.

Then run truffle migrate optionally including --network <networkName> and/or --reset or --f 3 (the latter just runs the final migration).

Actual results at the end of the final migration after doing once with reset and once with just the final migration:

Calling web3.eth.abi.decodeLog( [ { indexed: true,
internalType: 'int40',
name: 'howMuchAdded',
type: 'int40' },
{ indexed: true,
internalType: 'uint32',
name: 'newTotal',
type: 'uint32' } ] , 0x , [ '0x75da3623494ca2623e2d54ea6722f079f1867584dd0f44db9da1d6a3f18c609a',
'0x0000000000000000000000000000000000000000000000000000000000000010',
'0x0000000000000000000000000000000000000000000000000000000000000020' ] ) produced Result {
'0': '-395379449702',
'1': '16',
length: 2,
howMuchAdded: '-395379449702',
newTotal: '16' }
Added -395379449702 to the aggregator; the new total is 16.

With the patch applied, running the final migration again produces the expected results:

Calling web3.eth.abi.decodeLog( [ { indexed: true,
internalType: 'int40',
name: 'howMuchAdded',
type: 'int40' },
{ indexed: true,
internalType: 'uint32',
name: 'newTotal',
type: 'uint32' } ] , 0x , [ '0x75da3623494ca2623e2d54ea6722f079f1867584dd0f44db9da1d6a3f18c609a',
'0x0000000000000000000000000000000000000000000000000000000000000010',
'0x0000000000000000000000000000000000000000000000000000000000000030' ] ) produced Result {
'0': '16',
'1': '48',
length: 2,
howMuchAdded: '16',
newTotal: '48' }
Added 16 to the aggregator; the new total is 48.

(Note: 16*3=48).

The cause of the issue becomes clear if you inspect the values of parameters passed to ABICoder.prototype.decodeParameters.

[wbt]

The simplest workaround is, just before calling decodeLog, to call log.topics.shift();.

Unfortunately, if anybody is already using that workaround and the patch is applied, the result is an error:

Error: Returned values aren't valid, did it run Out of Gas? You might also see this error if you are not using the correct ABI for the contract you are retrieving data from, requesting data from a block number that does not exist, or querying a node which is not fully synced.

[spacesailor24]

I ran through the replication steps for verification, and the provided change does seem to correct the issue

Before patch:

After:

[github-actions]

This PR has been automatically marked as stale beacause it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions. If you believe this was a mistake, please comment.