safe-core-sdk icon indicating copy to clipboard operation
safe-core-sdk copied to clipboard

fix(protocol kit): Get modules paginated incorrect interface

Open leonardotc opened this issue 2 months ago • 2 comments

What it solves

The method getModulesPaginated should return a tuple containing both the modules as well as the next address of the page.

How this PR fixes it

  • It changes the interface by adding the missing property as a "complex type".
  • It adds the type since that is the result of the interface
  • It adds assertions to test that scenario

leonardotc avatar Apr 26 '24 12:04 leonardotc

Pull Request Test Coverage Report for Build 8848383373

Details

  • 230 of 248 (92.74%) changed or added relevant lines in 10 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+5.2%) to 78.812%

Changes Missing Coverage Covered Lines Changed/Added Lines %
packages/relay-kit/src/packs/safe-4337/utils.ts 8 10 80.0%
packages/relay-kit/src/packs/gelato/GelatoRelayPack.ts 6 11 54.55%
packages/relay-kit/src/packs/safe-4337/Safe4337Pack.ts 146 157 92.99%
<!-- Total: 230 248
Totals Coverage Status
Change from base Build 8602216308: 5.2%
Covered Lines: 704
Relevant Lines: 838

💛 - Coveralls

coveralls avatar Apr 26 '24 12:04 coveralls

This was added to the 1.4.1 of the contract (https://github.com/safe-global/safe-smart-account/blob/v1.4.1/contracts/base/ModuleManager.sol:

        /**
          Because of the argument validation, we can assume that the loop will always iterate over the valid module list values
          and the `next` variable will either be an enabled module or a sentinel address (signalling the end). 
          
          If we haven't reached the end inside the loop, we need to set the next pointer to the last element of the modules array
          because the `next` variable (which is a module by itself) acting as a pointer to the start of the next page is neither 
          included to the current page, nor will it be included in the next one if you pass it as a start.
        */
        if (next != SENTINEL_MODULES) {
            next = array[moduleCount - 1];
        }

leonardotc avatar May 02 '24 07:05 leonardotc

Beautiful @leonardotc thanks!. I think you need to pull from the target branch for fixing the failing tests

yagopv avatar May 08 '24 10:05 yagopv