Skip to content

Add optional path for blocklist config #8326

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

Merged
merged 13 commits into from
Sep 28, 2023

Conversation

MiroslavGatsanoga
Copy link
Contributor

@MiroslavGatsanoga MiroslavGatsanoga commented Sep 1, 2023

Description:

Currently the blocklist is defined in as resource and if we want to update the definition we will need to rebuild in order for the changes to take effect. That means local node is not able to use a blocklist definition different than the one defined under src/main/resources/evm-addresses-blocklist.csv.

This PR adds the ability to read the blocklist file specified by accounts.blocklist.path (renaming of the previous accounts.blocklist.resource) from some path e.g. data/onboard/evm-addresses-blocklist.csv. In order to achieve that the bootstrap.properties would need to contain the following two lines:

...
accounts.blocklist.enabled=true
accounts.blocklist.path=data/onboard/evm-addresses-blocklist.csv
...

If the path does not correspond to an existing file or it's left empty then we'll fallback and load the default src/main/resources/evm-addresses-blocklist.csv. An example config that would load the default resource would be:

...
accounts.blocklist.enabled=true
accounts.blocklist.path=
...

Related issues:

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

Signed-off-by: Miroslav Gatsanoga <[email protected]>
@MiroslavGatsanoga MiroslavGatsanoga marked this pull request as ready for review September 1, 2023 13:13
@MiroslavGatsanoga MiroslavGatsanoga requested a review from a team September 1, 2023 13:13
@MiroslavGatsanoga MiroslavGatsanoga requested a review from a team as a code owner September 1, 2023 13:13
Signed-off-by: Miroslav Gatsanoga <[email protected]>
@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Node: Unit Test Results

    2 218 files      2 218 suites   1h 20m 21s ⏱️
118 010 tests 117 976 ✔️ 34 💤 0
126 308 runs  126 274 ✔️ 34 💤 0

Results for commit 6073985.

♻️ This comment has been updated with latest results.

@github-actions
Copy link

github-actions bot commented Sep 1, 2023

Node: E2E Test Results

    1 files      1 suites   19m 10s ⏱️
310 tests 310 ✔️ 0 💤 0
328 runs  328 ✔️ 0 💤 0

Results for commit 6073985.

♻️ This comment has been updated with latest results.

Nana-EC
Nana-EC previously approved these changes Sep 1, 2023
povolev15
povolev15 previously approved these changes Sep 5, 2023
@github-actions
Copy link

github-actions bot commented Sep 11, 2023

Node: Integration Test Results

278 tests   278 ✔️  32m 35s ⏱️
    5 suites      0 💤
    5 files        0

Results for commit 6073985.

♻️ This comment has been updated with latest results.

iwsimon
iwsimon previously approved these changes Sep 19, 2023
Copy link
Contributor

@iwsimon iwsimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@github-actions
Copy link

github-actions bot commented Sep 20, 2023

Node: HAPI Test Results

1 187 tests   362 ✔️  23m 3s ⏱️
   161 suites  825 💤
   161 files        0

Results for commit 6073985.

♻️ This comment has been updated with latest results.

Signed-off-by: Miroslav Gatsanoga <[email protected]>
jeromy-cannon
jeromy-cannon previously approved these changes Sep 25, 2023
@mustafauzunn mustafauzunn added the Limechain Work planned for the LimeChain team label Sep 26, 2023
Signed-off-by: Miroslav Gatsanoga <[email protected]>
@codecov
Copy link

codecov bot commented Sep 26, 2023

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (c82334a) 65.27% compared to head (6073985) 65.27%.

Additional details and impacted files
@@              Coverage Diff              @@
##             develop    #8326      +/-   ##
=============================================
- Coverage      65.27%   65.27%   -0.01%     
  Complexity     29207    29207              
=============================================
  Files           3218     3219       +1     
  Lines         122314   122332      +18     
  Branches       12512    12514       +2     
=============================================
+ Hits           79845    79856      +11     
- Misses         39520    39525       +5     
- Partials        2949     2951       +2     
Files Coverage Δ
...e/mono/context/properties/BootstrapProperties.java 97.53% <100.00%> (ø)
...ate/initialization/BlocklistNotFoundException.java 100.00% <100.00%> (ø)
.../state/initialization/BlocklistAccountCreator.java 89.71% <65.38%> (-10.29%) ⬇️

... and 6 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jsync-swirlds
jsync-swirlds previously approved these changes Sep 27, 2023
Signed-off-by: Miroslav Gatsanoga <[email protected]>
@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Contributor

@iwsimon iwsimon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@Neeharika-Sompalli Neeharika-Sompalli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ! But seems default path is not set. Not sure if that is intentional

@MiroslavGatsanoga MiroslavGatsanoga merged commit a90c27c into develop Sep 28, 2023
@MiroslavGatsanoga MiroslavGatsanoga deleted the add-optional-blocklist-config-path branch September 28, 2023 14:04
@bibitibooo1 bibitibooo1 removed the Limechain Work planned for the LimeChain team label Oct 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants