feat: validações e emits em ValidatorSelection#2
feat: validações e emits em ValidatorSelection#2devfelipenunes wants to merge 1 commit intoRBBNet:feature/bdd-featuresfrom
Conversation
Adiciona/ajusta validações e emissões de eventos no contrato ValidatorSelection para melhorar integridade de estado e observabilidade. Arquivo afetado: src/ValidatorSelection.sol
rayangustavo
left a comment
rayangustavo
left a comment
There was a problem hiding this comment.
As validações e emissões de eventos são muito importantes e realmente estavam faltando. Acho que podemos separar o PR só para tratar disso.
Neste tem alteração de formatação, alteração da lógica de uso do lastBlockProposedBy, adição de novas funções para gerenciar os validadores operacionais e elegíveis, novas funções para ler o estado do contrato, além das validações e emissões que era o proposto inicial.
| import {UUPSUpgradeable} from "@openzeppelin/contracts/proxy/utils/UUPSUpgradeable.sol"; | ||
| import {OwnableUpgradeable} from "@openzeppelin-upgradeable/contracts/access/OwnableUpgradeable.sol"; | ||
|
|
||
| contract ValidatorSelection is IValidatorSelection, Initializable, Governable, OwnableUpgradeable, UUPSUpgradeable { |
There was a problem hiding this comment.
Acho que pode ser útil manter o IValidatorSelection para scripts futuros.
| address[] memory initialElegibleValidators | ||
| ) internal { | ||
| uint256 initialElegibleValidatorsLength = initialElegibleValidators | ||
| .length; |
There was a problem hiding this comment.
Eu manteria o ".length" na mesma linha, acho que neste caso a quebra de linha pode atrapalhar a leitura do código.
| .length; | ||
| if (initialElegibleValidatorsLength < MIN_NUMBER_OF_VALIDATORS) | ||
| revert FewEligibleValidators(); | ||
| for (uint256 i; i < initialElegibleValidatorsLength; i++) { |
There was a problem hiding this comment.
A lógica deste for loop é similar a das funções de adição de validador elegível, ao invés de repetir o mesmo código aqui, podemos chamar uma destas funções.
| _monitorsValidators(proposer, blockNumber); | ||
| if (_isAtSelectionBlock(blockNumber)) { | ||
| address[] memory selectedValidators = _selectValidators(blockNumber); | ||
| address[] memory selectedValidators = _selectValidators( |
There was a problem hiding this comment.
Acredito que aqui também seja melhor deixar na mesma linha. Se a função (ou análogo) recebe apenas um parâmetro e a linha não fica muito grande, então podemos deixar na mesma linha.
| uint256 numberOfOperationalValidators = operationalValidators.length(); | ||
| address[] memory auxArray = new address[](numberOfOperationalValidators); | ||
| address[] memory auxArray = new address[]( | ||
| numberOfOperationalValidators |
There was a problem hiding this comment.
Acredito que aqui também seja melhor deixar na mesma linha. Se a função (ou análogo) recebe apenas um parâmetro e a linha não fica muito grande, então podemos deixar na mesma linha.
| if (!operationalValidators.contains(validator)) | ||
| revert NotOperationalNode(validator); | ||
| operationalValidators.remove(validator); | ||
| delete lastBlockProposedBy[validator]; |
There was a problem hiding this comment.
Similar ao que foi comentado aqui: https://github.com/RBBNet/selecao-validadores/pull/2/changes#r2885125487
| if (!operationalValidators.contains(validator)) | ||
| revert NotOperationalNode(validator); | ||
| operationalValidators.remove(validator); | ||
| delete lastBlockProposedBy[validator]; |
There was a problem hiding this comment.
Similar ao que foi comentado aqui: https://github.com/RBBNet/selecao-validadores/pull/2/changes#r2885125487
| error InvalidConfigurationValue(string parameter, uint256 value); | ||
| error InvalidValidatorAddress(address validator); | ||
| error DuplicateValidator(address validator); | ||
| error CannotModifyDuringSelection(); |
There was a problem hiding this comment.
Esse custom error não está sendo usado.
| error InvalidValidatorAddress(address validator); | ||
| error DuplicateValidator(address validator); | ||
| error CannotModifyDuringSelection(); | ||
| error CannotRemoveOperationalValidator(address validator); |
There was a problem hiding this comment.
Esse custom error não está sendo usado.
| event EligibleValidatorRemoved(address indexed validator); | ||
| event OperationalValidatorAdded( | ||
| address indexed validator, | ||
| uint256 initialBlock |
There was a problem hiding this comment.
O initialBlock está sendo considerado aqui devido ao uso errado do lastBlockProposedBy, conforme descrito aqui: https://github.com/RBBNet/selecao-validadores/pull/2/changes#r2885214171
Então, fazendo a alteração descrita acima, acredito que o initialBlock deva ser removido.
2 participants
Adiciona/ajusta validações e emissões de eventos no contrato ValidatorSelection para melhorar integridade de estado e observabilidade. Arquivo afetado: src/ValidatorSelection.sol