add getters #12903
add getters #12903
Conversation
|
I see you updated files related to
|
|
I see you updated files related to |
shileiwill
force-pushed
the
enumberableSet
branch
from
03a7880
to
af3b5db
Compare
|
| function getTransmittersWithPayees() external view returns (TransmitterPayeeInfo[] memory) { | ||
| uint256 transmitterCount = s_transmittersList.length; | ||
| TransmitterPayeeInfo[] memory transmitters = new TransmitterPayeeInfo[](transmitterCount); | ||
|
|
||
| for (uint256 i = 0; i < transmitterCount; i++) { | ||
| address transmitterAddress = s_transmittersList[i]; | ||
| address payeeAddress = s_transmitterPayees[transmitterAddress]; | ||
|
|
||
| transmitters[i] = TransmitterPayeeInfo(transmitterAddress, payeeAddress); | ||
| } | ||
|
|
||
| return transmitters; | ||
| } | ||
| } |
There was a problem hiding this comment.
Hey sorry to do this after you've rebased.... but do you think we could change this into...
function getTransmitters() returns(address[]) {...}
function getPayees() returns(address[]) {...}This is derived from our effort to avoid "monolithic getters" and instead prefer individual getters. Doing this tends to avoid migration issues better. Ex if a future version of automaition adds a struct to TransmitterPayeeInfo like maybe they add the field balance, then anyone using getTransmittersWithPayees() will break when they upgrade to the new registry. If we force all consumers to use these individual getters, it makes migrating easier.
There was a problem hiding this comment.
No problem, we work to improve whatever can be improved :). I get your point, and totally agree with the benefit of having smaller getters.
The only counter opinion for this particular usecase is that the transmitter and payee is 1:1, and the order really matters. So having them together have the benefit of grouping things in the right order. Not a strong opinion tho, they are sort/count manually with two getters. WDYT?
There was a problem hiding this comment.
I'm thinking more about the lifetime of this struct, TransmitterPayeeInfo - we could be careful not to change it and maybe just add a comment. But it does remind me of the Upkeep struct where we add things to it version after version and now we have that whole UpkeepInfoLegacyV2PlusBlahBlah lol
There was a problem hiding this comment.
If we need to know more information about transmitters, I'd lean towards organizing that info into individual getters
There was a problem hiding this comment.
discussed offline, keeping it as is bc of the 1:1 mapping (order matters) of transmitter and payee.
https://smartcontract-it.atlassian.net/browse/AUTO-10211
https://smartcontract-it.atlassian.net/browse/AUTO-10210
https://smartcontract-it.atlassian.net/browse/AUTO-10207
https://smartcontract-it.atlassian.net/browse/AUTO-10206