Skip to content
This repository has been archived by the owner on Jun 11, 2024. It is now read-only.

Code improvements PoA module #2 #8964

Closed
Tracked by #7245
ishantiw opened this issue Sep 13, 2023 · 0 comments
Closed
Tracked by #7245

Code improvements PoA module #2 #8964

ishantiw opened this issue Sep 13, 2023 · 0 comments
Assignees
Milestone

Comments

@ishantiw
Copy link
Contributor

ishantiw commented Sep 13, 2023

Description

  1. RegisterAuthorityParams fields are ordered differently than in the LIP:
    The generatorKey and proofOfPossession fields are swapped. This could potentially cause schema matching problems if JSON serialization is done in the wrong order. However, we did not find any security problems related to this issue.

  2. Unnecessary calls to validator.validate:
    In the PoA module’s register authority and [update generator key](https://github.com/LiskHQ/lisk-sdk/blob/ed5649eb954c7c47e11eb2d2ea2b84b9336c4c4b/framework/src/modules/poa/commands/update_generator_key.ts#L4

  1. commands, the code calls validator.validate to validate the commands parameters. Calling validator.validate on a command's verify function is unnecessary because this check is already done in the state machine logic.
  1. Array ordering is checked manually when a helper function could be used:
    In the PoAModule.initGenesisState function, there is a check for whether the validatorAddresses list is sorted, by comparing each of its elements against the corresponding element in [...validatorAddresses].sort(). Instead, the objectUtils.isBufferArrayOrdered helper function should be used. The same applies to the check done later in the function on the activeValidators list: objectUtils.isBufferArrayOrdered(activeValidatorAddresses) should be used.

  2. The shuffleValidatorList function is defined both in PoA and PoS:
    Both do essentially the same thing, but are implemented in slightly different ways. Ideally a common implementation should be used by both.

Steps to reproduce

N/A

Which version(s) does this affect? (Environment, OS, etc...)

6.1.0-beta.1

@ishantiw ishantiw changed the title Code improvements PoA module Code improvements PoA module #2 Sep 13, 2023
@Phanco Phanco self-assigned this Sep 13, 2023
@Madhulearn Madhulearn added this to the Sprint 104 milestone Sep 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants