add getter, add comment and add field in event

[shileiwill]

As title, minor improvements on both registry and registrar

[RyanRHall]

One small suggestion

[RyanRHall]

since we have to change this event anyway.. I think we could better "future proof" it by changing it to just include the whole RegistrationParams struct. wdyt?

event RegistrationRequested(
    bytes32 indexed hash,
    RegistrationParams registrationParams
)

[shileiwill]

Using registrationParams is far neater, I like it! Pasting it here for reference:

  struct RegistrationParams {
    address upkeepContract;
    uint96 amount;
    // 1 word full
    address adminAddress;
    uint32 gasLimit;
    uint8 triggerType;
    // 7 bytes left in 2nd word
    IERC20 billingToken;
    // 12 bytes left in 3rd word
    string name;
    bytes encryptedEmail;
    bytes checkData;
    bytes triggerConfig;
    bytes offchainConfig;
  }

I confirmed registrationParams contains exactly the same the params needed, except hash, which we will add.

For an Event (RegistrationRequested) which has a Struct (RegistrationParams), the hash will be just listing all the fields in the struct, so keccak(bytes32,address upkeepContract,uint96 amount,address....)?

I believe that is how it works, but double confirm @RyanRHall ?
We will need to function signature in atlas, hence confirming, but I dont think using the RegistrationParams will require any other changes except the hash. cc @cmalec

[cmalec]

ya for new versions of the event we will just need to decode RegistrationParams but should be fine.

[shileiwill]

Note to myself, the event signature is now:

• RegistrationRequested(bytes32,address,uint96,address,uint32,uint8,address,string,bytes,bytes,bytes,bytes)
• 0x72319f308e224802c7dc5f4bad21a9b46070f765d25eedf39a25f04bf39eb632

[shileiwill]

I realized that we introduced some side effects bc of this change. Given that we dont change the params in the event often, I reverted this change. More details in the other comment.

[cl-sonarqube-production]

Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

[RyanRHall]

LGTM 👍

[shileiwill]

Hey @RyanRHall / @FelixFan1992 : this refactoring results in a feature change. This upkeepContract is indexed and address is recommended to be indexed, but when we wrap the params with the RegistrationParams struct, we lose this indexed address.

Not a strong opinion but I would recommend keeping the original structure to have the contract indexed and this old structure can also make the atlas changes easier/cleaner (have to replicate the struct in test).