Review what would happen if hundreds or thousands of modules were added to ModuleRegistry about polymath-core HOT 8 CLOSED

Hi @pabloruiz55,

So - I spent a while last night playing with this, with the following conclusions.

1. It is possible to specify a gas limit higher than the block gas limit when making a call (rather than sending a transaction), but there are some challenges:

• testrpc doesn't support these semantics - if you specify a high gas price on a call, it will return a "Exceeds Block Gas Limit" error.

AFAICT it is a client side implementation (e.g. Geth, Parity, testrpc) as to whether a high gas price for calls that exceeds the block gas limit is allowed or not.

2. For our use case of returning a large list (e.g. moduleList for a given moduleType) this isn't really a problem as the gas required to do this seems to be very low. My understanding is that when calling a function that returns a contract, on-chain, you need gas to reserve memory for the returned data, however when calling from web3 (rather than another contract) this doesn't apply, so only the gas required by the actual function (which is tiny) is needed. I tried adding 1000 modules and was still able to return the data fine.

If you take a look at the test case:
Should add lots of modules to module list, and remain useable in module_registry.js, in the branch new-test-for-moduleRegistry it does this. You may need to comment out other tests to ensure this test has enough ETH balance on the sending account to run.

I wouldn't suggest merging this branch with master as the test case is slow (as it creates and adds 1000 modules).

3. Using web3 directly, from a browser web3 session (i.e. using the MetaMask injected web3 object) it is possible to check this behaviour. I deployed the following simple contract to Ropsten at address 0x98914d531caa2765c478ec52cd6c16185e9ead4d.

pragma solidity ^0.4.21;
contract GasTest {
  uint256 public temp;
  function doWork(uint256 loops) public returns (uint256) {
    for (uint256 i = 0; i < loops; i++) {
      temp = temp + i;
    }
    return temp;
  }
}

Calling doWork(10000) as a transaction (on Ropsten) fails with an Out of Gas error - e.g.:
https://ropsten.etherscan.io/tx/0xfe95eb83600b9a49f73c4dc536a55f2fd30b6c12864c00016ee278be23a1023b

Calling doWork(10000) as a call (rather than transaction) works regardless of what value you pass to doWork.

To test this, you can (in a browser javascript console, with MetaMask running and on Ropsten), execute the following:

const gasTestABI = [{"constant":true,"inputs":[],"name":"temp","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"view","type":"function"},{"constant":false,"inputs":[{"name":"loops","type":"uint256"}],"name":"doWork","outputs":[{"name":"","type":"uint256"}],"payable":false,"stateMutability":"nonpayable","type":"function"}];
const gasTestAddress = "0x98914d531caa2765c478ec52cd6c16185e9ead4d";//Address on Ropsten
let contract = web3.eth.contract(gasTestABI).at(gasTestAddress);
//Following runs and returns the sum of 1 to 10,000:
contract.doWork.call(100000, (err, data) => console.log(data.toString()));
//Following will submit a transaction, and fails with out of gas:
contract.doWork(100000, {gas: 4500000}, (err, data) => console.log(data.toString()));

4. If you want to, you can run a similar test against Ropsten, using e.g. Geth. The script gas_test.js in the branch new-test-for-moduleRegistry does this. You need to have Geth running against Ropsten on 8545. The first send fails with out of gas, and the second call (with a very high gas limit) succeeds and returns. You can run this with:
   node test/gas_test.js

It may still be that being able to remove modules is useful functionality though, although arguably the logic around only being able to use verified modules (or modules the issuer themselves own) guards against this. The UI could show only those modules which fulfil these criteria to an issue, which should remove any uninteresting modules from their view.

from polymath-core.

So - moduleList and reputation are meant to be read only via calls (rather than transactions), as the size the corresponding arrays can grow to is unbounded. When making a call, you are not subject to the block gas limit (so can set e.g. 50M as the gas limit) so reads should be fine.

With regards to unRegistering modules, we can add this functionality (which would require some additional data structures to avoid having to loop over all modules of a given type) - my previous thinking was that if PolyMath didn't "like" a module factory, it would just unverify it rather than removing it all together (so that the registry is still somewhat decentralised for unverified modules, and PM controls which modules are verified).

Let me know what you think.

from polymath-core.

openethereum/parity-ethereum#6293

from polymath-core.

I was under the impression that you would still be subject to gas limits even if it's just a call.
Can you please create a test for this? I'd like to see it working just to be sure.

If that's the case, why not store the whitelist data in an array as well and retrieve all records?
Right now we are using events but it's a bit cumbersome as you have to filter old events for a given address.

from polymath-core.

I will create a test case for this to double-check, but the above thread seems to indicate that whilst you are subject to gas limits, you are not subject to the block gas limit.

For e.g. whitelists, the issue is if you want to modify an item you need to avoid looping through the whole loop, so you need to keep a pointer to the position on the list which makes mappings easier IMO.

from polymath-core.

I'm very interested in seeing this work (or not work if that's the case).

from polymath-core.

"still somewhat decentralised for unverified modules" I like this notion, that anyone can add them w/o our permission, yet we keep 'verified' ones. Is there a way to have modules classified by a person's registered domain? I had seen an eth-domain system getting going as an EIP.

It seems that you would eventually want a remove functionality - possibly for unknown circumstances that we can't imagine right now.

from polymath-core.

Integration of ENS (eth domains) into the Protocol is def. an interesting topic - possibly adding a module for this would be a great hackathon entry! (i.e. a module to allow an issuer to "easily" register an ENS domain for their security token).

from polymath-core.