Update ModuleManager.sol

[mytudan]

Based on the setupModules and enableModule methods in the ModuleManager, we can ascertain that SENTINEL_MODULES points to the head node. The linked list can be represented as follows: SENTINEL_MODULES --> newModule --> ModuleA --> ModuleB --> ModuleC --> address(0x01).

Given this, isn't there redundancy in the getModulesPaginated method? For example, the check next != SENTINEL_MODULES seems redundant because next can never be SENTINEL_MODULES as it points to the head node. Moreover, the first line of the getModulesPaginated method already ensures that start != SENTINEL_MODULES.

[github-actions]

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

[mytudan]

I have read the CLA Document and I hereby sign the CLA

[remedcu]

The linked list can be represented as follows: SENTINEL_MODULES --> newModule --> ModuleA --> ModuleB --> ModuleC --> address(0x01).

The SENTINEL_MODULES is address(0x1): https://github.com/safe-global/safe-smart-account/pull/792/files#diff-82762908b9416ddadffb149ee4d25f328078fc27f938d454d8a207aad1ec3839L59

the check next != SENTINEL_MODULES seems redundant because next can never be SENTINEL_MODULES as it points to the head node

So, the next (as it moves forward within the while loop) can become SENTINEL_MODULES eventually if the pageSize is greater than the remaining module list from the start.

You can see the tests failing here: https://github.com/safe-global/safe-smart-account/actions/runs/9949739616/job/27494234482?pr=792

Also, to check further with your implementation locally, you can run the test as mentioned in the README: https://github.com/safe-global/safe-smart-account/tree/main?tab=readme-ov-file#testing

[nlordell]

I think the proposed changes won't work. In particular, the empty module set is a linked list that looks like: SENTINEL -> SENTINEL. This means that, without the condition in the while loop, it will iterate forever.