Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Validation errors (duplicated account) ignored in accounts-allowlist and empty list returned #7138

Closed
aiza-fp opened this issue May 23, 2024 · 1 comment
Labels
bug Something isn't working good first issue Good for newcomers non mainnet (private networks) not related to mainnet features - covers privacy, permissioning, IBFT2, QBFT P5 Very Low (ex: JSON-RPC json object response not ignoring null keys, typo on a CLI description)

Comments

@aiza-fp
Copy link

aiza-fp commented May 23, 2024

Description

When an initial accounts-allowlist is defined, no exception is thrown if there are duplicated accounts. Other input validation errors are being ignored too.

Acceptance Criteria

  • If a validation error occurs an exception is thrown

Steps to Reproduce (Bug)

  1. Define a accounts-allowlist in the default permissions file or custom file where a duplicated account can be found.
  2. Start the node and use the PERM API to perm_getAccountsAllowlist (see example below).
  3. The result is an empty list (see example below).

Expected behavior: Get an exception that stops the node or ignore the duplicated account.

Actual behavior: It returns an empty list and it doesn't show a warning or throw an exception.

Frequency: 100%

Logs (if a bug)

No logs

Versions (Add all that apply)

  • Software version: 24.5.1
  • Java version: openjdk-java-21
  • OS Name & Version: linux-x86_64
  • Docker Version: 26.0.0

Additional Information - Example:

WORKS FINE:
accounts-allowlist=["0x4c3be0df1d9ff62856b721c13cefd1721b383bdc","0x432132e8561785c33afe931762cf8eeb9c80e3ad","0xcb88953e60948e3a76fa658d65b7c2d5043c6409","0xdd76406b124f9e3ae9fbeb47e4d8dc0ab143902d"]

RESULT:
$ curl -X POST --data '{"jsonrpc":"2.0","method":"perm_getAccountsAllowlist","params":[], "id":1}' http://127.0.0.1:8545
{"jsonrpc":"2.0","id":1,"result":["0x4c3be0df1d9ff62856b721c13cefd1721b383bdc","0x432132e8561785c33afe931762cf8eeb9c80e3ad","0xcb88953e60948e3a76fa658d65b7c2d5043c6409","0xdd76406b124f9e3ae9fbeb47e4d8dc0ab143902d"]}

MODIFIED, WITH DUPLICATED ACCOUNT
accounts-allowlist=["0x4c3be0df1d9ff62856b721c13cefd1721b383bdc","0x432132e8561785c33afe931762cf8eeb9c80e3ad","0xcb88953e60948e3a76fa658d65b7c2d5043c6409","0x432132E8561785c33Afe931762cf8EEb9c80E3aD"]

RESULT:
$ curl -X POST --data '{"jsonrpc":"2.0","method":"perm_getAccountsAllowlist","params":[], "id":1}' http://127.0.0.1:8545
{"jsonrpc":"2.0","id":1,"result":[]}

The bug is in the file AccountLocalConfigPermissioningController.java, methods readAccountsFromConfig and addAccounts because they are not taking into account the result returned by addAccounts. Errors returned by inputValidation are ignored.

@macfarla macfarla added P5 Very Low (ex: JSON-RPC json object response not ignoring null keys, typo on a CLI description) bug Something isn't working non mainnet (private networks) not related to mainnet features - covers privacy, permissioning, IBFT2, QBFT good first issue Good for newcomers labels May 29, 2024
macfarla pushed a commit that referenced this issue Jun 4, 2024
…#7138] (#7161)

* Error out on permissions config accounts-allowlist validation errors.

Signed-off-by: krishnannarayanan <krsh24@gmail.com>

* Fixing compilation errors

Signed-off-by: krishnannarayanan <krsh24@gmail.com>

* Incorrect file check in

Signed-off-by: krishnannarayanan <krsh24@gmail.com>

---------

Signed-off-by: krishnannarayanan <krsh24@gmail.com>
@macfarla
Copy link
Contributor

fixed in #7161

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers non mainnet (private networks) not related to mainnet features - covers privacy, permissioning, IBFT2, QBFT P5 Very Low (ex: JSON-RPC json object response not ignoring null keys, typo on a CLI description)
Projects
None yet
Development

No branches or pull requests

2 participants