Registries - Incomplete input validation and inconsistent order of validations  Pending
Methods and Functions usually live in one of two worlds:\\n`public` API - methods declared with visibility `public` or `external` exposed for interaction by other parties\\n`internal` API - methods declared with visibility `internal`, `private` that are not exposed for interaction by other parties\\nWhile it is good practice to visually distinguish internal from public API by following commonly accepted naming convention e.g. by prefixing internal functions with an underscore (_doSomething vs. doSomething) or adding the keyword `unsafe` to `unsafe` functions that are not performing checks and may have a dramatic effect to the system (_unsafePayout vs. RequestPayout), it is important to properly verify that inputs to methods are within expected ranges for the implementation.\\nInput validation checks should be explicit and well documented as part of the code's documentation. This is to make sure that smart-contracts are robust against erroneous inputs and reduce the potential attack surface for exploitation.\\nIt is good practice to verify the methods input as early as possible and only perform further actions if the validation succeeds. Methods can be split into an external or public API that performs initial checks and subsequently calls an internal method that performs the action.\\nThe following lists some public API methods that are not properly checking the provided data:\\n`BlockhashRegistry.reCalculateBlockheaders` - bhash can be zero; blockheaders can be empty\\nBlockhashRegistry.getParentAndBlockhash- blockheader structure can be random as long as parenthash can be extracted\\n`BlockhashRegistry.recreateBlockheaders` - blockheaders can be empty; Arguments should be validated before calculating values that depend on them:\\n```\\nassert(\\_blockNumber > \\_blockheaders.length);\\n```\\n\\n`BlockhashRegistry.searchForAvailableBlock` - `_startNumber + _numBlocks` can be > `block.number; _startNumber + _numBlocks` can overflow.\\n`NodeRegistry.removeNode` - should check `require(_nodeIndex < nodes.length)` first before any other action.\\n```\\nfunction removeNode(uint \\_nodeIndex) internal {\\n    // trigger event\\n    emit LogNodeRemoved(nodes[\\_nodeIndex].url, nodes[\\_nodeIndex].signer);\\n    // deleting the old entry\\n    delete urlIndex[keccak256(bytes(nodes[\\_nodeIndex].url))];\\n    uint length = nodes.length;\\n\\n    assert(length > 0);\\n```\\n\\n`NodeRegistry.registerNodeFor` - Signature version `v` should be checked to be either `27 || 28` before verifying it.\\n```\\nfunction registerNodeFor(\\n    string calldata \\_url,\\n    uint64 \\_props,\\n    uint64 \\_timeout,\\n    address \\_signer,\\n    uint64 \\_weight,\\n    uint8 \\_v,\\n    bytes32 \\_r,\\n    bytes32 \\_s\\n)\\n    external\\n    payable\\n{\\n```\\n\\n`NodeRegistry.revealConvict` - unchecked `signer`\\n```\\nSignerInformation storage si = signerIndex[\\_signer];\\n```\\n\\n`NodeRegistry.revealConvict` - signer status can be checked earlier.\\n```\\nrequire(si.stage != Stages.Convicted, "node already convicted");\\n```\\n\\n`NodeRegistry.updateNode` - the check if the `newURL` is registered can be done earlier\\n```\\nrequire(!urlIndex[newURl].used, "url is already in use");\\n```\\n
Use Checks-Effects-Interactions pattern for all functions.
```\\nassert(\\_blockNumber > \\_blockheaders.length);\\n```\\n