Fix for removed owners confirmations being accounted

[udityadav-supraoracles]

This PR resolves the issue which takes into account confirmations from removed owners also.

[aregng]

@udityadav-supraoracles , can you please check the following PR: #33
it addresses this issue with different approach and confirmation clearing was not done as it will affect the performance/gas-usage.
Can we do the Benchmarking to have more realistic data to understand the actual need of clearing confirmation.

[udityadav-supraoracles]

@udityadav-supraoracles , can you please check the following PR: #33 it addresses this issue with different approach and confirmation clearing was not done as it will affect the performance/gas-usage. Can we do the Benchmarking to have more realistic data to understand the actual need of clearing confirmation.

Actually I cleared the confirmations in order to return correct data and not stale data for following view functions:

    function isConfirmed(uint256 _txIndex, address _owner)
    function getTransaction(uint256 _txIndex)

[aregng]

@udityadav-supraoracles , can you please check the following PR: #33 it addresses this issue with different approach and confirmation clearing was not done as it will affect the performance/gas-usage. Can we do the Benchmarking to have more realistic data to understand the actual need of clearing confirmation.

Actually I cleared the confirmations in order to return correct data and not stale data for following view functions:

    function isConfirmed(uint256 _txIndex, address _owner)
    function getTransaction(uint256 _txIndex)

I think performance comes first. These 2 functions can be updated to make sure that input _owner is still part of the valid owners before running the check, and return false if not. and the second one can be updated to filter out confirmations by owners which have been removed and return only the valid count.

    function validNumberOfConfirmations(uint256 _txIndex)  returns (uint64) {
        txExists(_txIndex);
        EnumerableSet.AddressSet storage confirmation = confirmations[_txIndex];
        uint64 valid_number_of_confirmations = 0;
        for (uint64 i = 0; i < confirmation.length(); i++) {
            address owner = confirmation.at(i);
            if (owners.contains(owner)) {
                valid_number_of_confirmations++;
            }
        }
        return valid_number_of_confirmations;
    }

[aregng]

Looks good, only small suggestion if it is acceptable.