feat: HIP-904 Introduce `unlimitedAutoAssociations` feature flag #13041

stoyanov-st · 2024-04-29T08:09:17Z

Description:
With this PR we are introducing the unlimitedAutoAssociations feature flag in EntitiesConfig.
We also add relative changes to handler logic in regards to auto associations and relative tests.
Related issue(s):

Fixes #12981

Notes for reviewer:

Checklist

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

github-actions · 2024-04-29T08:25:14Z

Node: HAPI Test (Restart) Results

2 tests 2 ✅ 8m 3s ⏱️
2 suites 0 💤
2 files 0 ❌

Results for commit d9d8314.

♻️ This comment has been updated with latest results.

github-actions · 2024-04-29T08:25:31Z

Node: HAPI Test (Node Death Reconnect) Results

2 tests 2 ✅ 10m 8s ⏱️
2 suites 0 💤
2 files 0 ❌

Results for commit d9d8314.

♻️ This comment has been updated with latest results.

github-actions · 2024-04-29T08:39:22Z

Node: HAPI Test (Token) Results

235 tests 233 ✅ 24m 24s ⏱️
17 suites 2 💤
17 files 0 ❌

Results for commit d9d8314.

♻️ This comment has been updated with latest results.

github-actions · 2024-04-29T08:48:54Z

Node: HAPI Test (Misc) Results

464 tests 454 ✅ 44m 4s ⏱️
77 suites 10 💤
77 files 0 ❌

Results for commit d9d8314.

♻️ This comment has been updated with latest results.

github-actions · 2024-04-29T08:58:00Z

Node: HAPI Test (Crypto) Results

335 tests 335 ✅ 39m 49s ⏱️
25 suites 0 💤
25 files 0 ❌

Results for commit d9d8314.

♻️ This comment has been updated with latest results.

github-actions · 2024-04-29T09:13:37Z

Node: HAPI Test (Time Consuming) Results

21 tests 21 ✅ 53m 48s ⏱️
3 suites 0 💤
3 files 0 ❌

Results for commit d9d8314.

♻️ This comment has been updated with latest results.

github-actions · 2024-04-29T09:27:35Z

Node: Unit Test Results

2 305 files ±0 2 305 suites ±0 4h 25m 8s ⏱️ + 1h 43m 32s
118 958 tests ±0 118 891 ✅ ±0 67 💤 ±0 0 ❌ ±0
127 519 runs ±0 127 452 ✅ ±0 67 💤 ±0 0 ❌ ±0

Results for commit d9d8314. ± Comparison against base commit e64faab.

This pull request removes 4026 and adds 3789 tests. Note that renamed tests count towards both.


  
             IssuerDN: CN=s-aaaa
            SubjectDN: CN=s-aaaa
           Final Date: Fri Jan 01 00:00:00 UTC 2100
           Public Key: RSA Public Key [2e:28:bc:1e:d3:83:25:92:8e:cb:98:b1:b6:84:06:9c:d5:d8:14:d5],[56:66:d1:a4]
           Start Date: Sat Jan 01 00:00:00 UTC 2000
         SerialNumber: 12482092706667292405
        modulus: c1a0ff5d2372b53d12d12bb87dd03f5e…
        modulus: c1a0ff5d2372b53d12d12bb87dd03f5…
…

com.hedera.node.app.grpc.impl.netty.GrpcServiceBuilderTest ‑ [4] 

com.hedera.node.app.grpc.impl.netty.GrpcServiceBuilderTest ‑ [6] 

com.hedera.node.app.grpc.impl.netty.GrpcServiceBuilderTest ‑ [7]   
  
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [10] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@90f80f0f
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [11] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@6e1a2a2b
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [12] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@2856a881
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [13] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@42fe80e9
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [14] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@982d7477
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [15] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@5f1be691
com.hedera.node.app.service.mono.state.codec.VirtualKeySerdesAdapterTest ‑ [16] com.hedera.node.app.service.mono.state.codec.VirtualBlobKey@ebb351ed
…

♻️ This comment has been updated with latest results.

github-actions · 2024-04-29T09:30:58Z

Node: HAPI Test (Smart Contract) Results

592 tests 592 ✅ 1h 15m 41s ⏱️
63 suites 0 💤
63 files 0 ❌

Results for commit d9d8314.

♻️ This comment has been updated with latest results.

MrValioBg · 2024-05-07T07:06:43Z

It seems like this PR includes adding the feature flag for both the mono and modularized codebase. Why do we need it in mono?

...ain/java/com/hedera/node/app/service/token/impl/handlers/TokenAssociateToAccountHandler.java

hedera-node/hedera-config/src/main/java/com/hedera/node/config/data/ContractsConfig.java

...pl/src/main/java/com/hedera/node/app/service/token/impl/validators/TokenCreateValidator.java

Neeharika-Sompalli · 2024-05-14T15:44:58Z

hedera-node/hedera-config/src/main/java/com/hedera/node/config/data/EntitiesConfig.java

@@ -24,4 +24,5 @@
 public record EntitiesConfig(
        @ConfigProperty(defaultValue = "3153600000") @NetworkProperty long maxLifetime,
        // @ConfigProperty(defaultValue = "FILE") Set<EntityType> systemDeletable
-        @ConfigProperty(defaultValue = "false") @NetworkProperty boolean limitTokenAssociations) {}
+        @ConfigProperty(defaultValue = "false") @NetworkProperty boolean limitTokenAssociations,
+        @ConfigProperty(defaultValue = "true") @NetworkProperty boolean unlimitedAutoAssociations) {}


Should this be unlimitedAutoAssociationsEnabled or allowUnlimitedAutoAssociations or enableUnlimitedAutoAssociations for clarity

I will change it to allowUnlimitedAutoAssociations

I think it should be unlimitedAutoAssociationsEnabled, because we use this pattern more frequently.

Neeharika-Sompalli · 2024-05-14T15:47:02Z

hedera-node/hedera-config/src/main/java/com/hedera/node/config/data/EntitiesConfig.java

@@ -24,4 +24,5 @@
 public record EntitiesConfig(
        @ConfigProperty(defaultValue = "3153600000") @NetworkProperty long maxLifetime,
        // @ConfigProperty(defaultValue = "FILE") Set<EntityType> systemDeletable
-        @ConfigProperty(defaultValue = "false") @NetworkProperty boolean limitTokenAssociations) {}
+        @ConfigProperty(defaultValue = "false") @NetworkProperty boolean limitTokenAssociations,
+        @ConfigProperty(defaultValue = "true") @NetworkProperty boolean unlimitedAutoAssociations) {}


Also this should be in TokensConfig

This new property is related to accounts. Is there a reason to move it to token configurations?

Neeharika-Sompalli · 2024-05-14T15:53:49Z

...l/src/main/java/com/hedera/node/app/service/token/impl/validators/CryptoCreateValidator.java

-        return n > ledgerConfig.maxAutoAssociations()
-                || (entitiesConfig.limitTokenAssociations() && n > tokensConfig.maxPerAccount());
+        return (entitiesConfig.limitTokenAssociations() && n > tokensConfig.maxPerAccount())
+                || (!entitiesConfig.unlimitedAutoAssociations() && n > ledgerConfig.maxAutoAssociations());


Why do we need to check !entitiesConfig.unlimitedAutoAssociations() this here ? Even if the configuration is enabled, we should not be able to set a higher number. That configuration is just allowed for -1 to be set in maxAutoAssociations right ?

From my understanding we should not limit the automatic associations if the configuration is enabled.
Or is it only if the value is -1 we have unlimited auto associations and we keep limiting the positive values e.g. 5000 is valid but 5001 - invalid?
@Neeharika-Sompalli @MiroslavGatsanoga Can you provide more info on this?

I agree. If we allow unlimited auto associations, it is hard to explain why setting a concrete number should be limited. But this changes the current behavior and therefore we have to double-check (and eventually update the HIP).

Or is it only if the value is -1 we have unlimited auto associations and we keep limiting the positive values e.g. 5000 is valid but 5001 - invalid

Yes, that should be the intended behaviour. Having the feature flag enabled should allow maxAutoAssociations value for any account to be set to -1.
If an account has maxAutoAssociations = -1 then we have unlimited auto associations for this account.

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

codecov · 2024-05-15T14:05:36Z

Codecov Report

Attention: Patch coverage is 50.00000% with 3 lines in your changes are missing coverage. Please review.

Project coverage is 61.74%. Comparing base (e64faab) to head (d9d8314).

Files	Patch %	Lines
...e/token/impl/validators/CryptoCreateValidator.java	50.00%	0 Missing and 2 partials ⚠️
...rvice/token/impl/handlers/CryptoUpdateHandler.java	50.00%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@                                Coverage Diff                                @@
##             012981-hip-904-unlimited-max-auto-associations   #13041   +/-   ##
=================================================================================
  Coverage                                             61.73%   61.74%           
- Complexity                                            31227    31231    +4     
=================================================================================
  Files                                                  3442     3442           
  Lines                                                141267   141269    +2     
  Branches                                              14778    14780    +2     
=================================================================================
+ Hits                                                  87217    87223    +6     
+ Misses                                                50163    50159    -4     
  Partials                                               3887     3887

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

stoyanov-st added the Limechain Work planned for the LimeChain team label Apr 29, 2024

stoyanov-st self-assigned this Apr 29, 2024

stoyanov-st requested review from MrValioBg and mustafauzunn May 7, 2024 06:31

MrValioBg reviewed May 7, 2024

View reviewed changes

...ain/java/com/hedera/node/app/service/token/impl/handlers/TokenAssociateToAccountHandler.java Outdated Show resolved Hide resolved

mustafauzunn reviewed May 9, 2024

View reviewed changes

hedera-node/hedera-config/src/main/java/com/hedera/node/config/data/ContractsConfig.java Outdated Show resolved Hide resolved

MrValioBg reviewed May 13, 2024

View reviewed changes

...pl/src/main/java/com/hedera/node/app/service/token/impl/validators/TokenCreateValidator.java Outdated Show resolved Hide resolved

stoyanov-st marked this pull request as ready for review May 13, 2024 14:47

stoyanov-st requested review from a team and tinker-michaelj as code owners May 13, 2024 14:48

stoyanov-st requested review from mhess-swl, acuarica, netopyr and Neeharika-Sompalli May 13, 2024 14:48

mustafauzunn linked an issue May 14, 2024 that may be closed by this pull request

HIP-904: Add feature flag for unlimited token associations. Modify TokenConfig and EntityConfig to ensure we are not limiting maxAutoAssociations. #12981

Open

Neeharika-Sompalli reviewed May 14, 2024

View reviewed changes

stoyanov-st requested a review from a team as a code owner May 15, 2024 10:42

stoyanov-st added 8 commits May 15, 2024 14:04

fix: adjust association checks

ba08163

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

fix: move unlimitedAutoAssociation to EntitiesConfig

8f35ae2

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

fix: adjust test

ad5a00b

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

fix: correct mock test

918e6d5

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

fix: remove unnecessary stubbing

96e0568

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

fix: override unlimitedAutoAssociations to ensure vanilla behaviour

3acc6dc

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

nit: remove unnecessary brackets

0fcff82

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

refactor: rename flag for clarity

7270b85

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

stoyanov-st force-pushed the 012981-hip-904-unlimited-max-auto-associations-feature-flag branch from a5f43f7 to 7270b85 Compare May 15, 2024 11:09

stoyanov-st removed request for a team, dalvizu, rbair23, poulok, Nana-EC, SimiHunjan, steven-sheehy, nathanklick, hendrikebbers, imalygin and OlegMazurov May 15, 2024 11:10

stoyanov-st added 4 commits May 15, 2024 15:14

fix: adjust logic for unlimitedAutoAssoc

56dcec2

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

nit: spotless

7fcde28

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

fix: validate negative values for maxAutoAssoc

6b652e3

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

test: adjust tests

d9d8314

Signed-off-by: Stanimir Stoyanov <stanimir.stoyanov@limechain.tech>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: HIP-904 Introduce `unlimitedAutoAssociations` feature flag #13041

feat: HIP-904 Introduce `unlimitedAutoAssociations` feature flag #13041

stoyanov-st commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

MrValioBg commented May 7, 2024

Neeharika-Sompalli May 14, 2024 •

edited

stoyanov-st May 15, 2024

netopyr May 15, 2024

Neeharika-Sompalli May 14, 2024

stoyanov-st May 15, 2024

Neeharika-Sompalli May 14, 2024

stoyanov-st May 15, 2024

netopyr May 15, 2024

MiroslavGatsanoga May 15, 2024 •

edited

codecov bot commented May 15, 2024

feat: HIP-904 Introduce unlimitedAutoAssociations feature flag #13041

Are you sure you want to change the base?

feat: HIP-904 Introduce unlimitedAutoAssociations feature flag #13041

Conversation

stoyanov-st commented Apr 29, 2024 • edited

github-actions bot commented Apr 29, 2024 • edited

Node: HAPI Test (Restart) Results

github-actions bot commented Apr 29, 2024 • edited

Node: HAPI Test (Node Death Reconnect) Results

github-actions bot commented Apr 29, 2024 • edited

Node: HAPI Test (Token) Results

github-actions bot commented Apr 29, 2024 • edited

Node: HAPI Test (Misc) Results

github-actions bot commented Apr 29, 2024 • edited

Node: HAPI Test (Crypto) Results

github-actions bot commented Apr 29, 2024 • edited

Node: HAPI Test (Time Consuming) Results

github-actions bot commented Apr 29, 2024 • edited

Node: Unit Test Results

github-actions bot commented Apr 29, 2024 • edited

Node: HAPI Test (Smart Contract) Results

MrValioBg commented May 7, 2024

Neeharika-Sompalli May 14, 2024 • edited

Choose a reason for hiding this comment

stoyanov-st May 15, 2024

Choose a reason for hiding this comment

netopyr May 15, 2024

Choose a reason for hiding this comment

Neeharika-Sompalli May 14, 2024

Choose a reason for hiding this comment

stoyanov-st May 15, 2024

Choose a reason for hiding this comment

Neeharika-Sompalli May 14, 2024

Choose a reason for hiding this comment

stoyanov-st May 15, 2024

Choose a reason for hiding this comment

netopyr May 15, 2024

Choose a reason for hiding this comment

MiroslavGatsanoga May 15, 2024 • edited

Choose a reason for hiding this comment

codecov bot commented May 15, 2024

Codecov Report

feat: HIP-904 Introduce `unlimitedAutoAssociations` feature flag #13041

feat: HIP-904 Introduce `unlimitedAutoAssociations` feature flag #13041

stoyanov-st commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

github-actions bot commented Apr 29, 2024 •

edited

Neeharika-Sompalli May 14, 2024 •

edited

MiroslavGatsanoga May 15, 2024 •

edited