Added getTerminusAdminInfo to DropperFacet #294
Conversation
There was a problem hiding this comment.
@ShivaLaguna : This is a much needed addition. Thank you for making the PR.
I have left a couple of comments - minor changes in the code, but it will help to keep it consistent with the other contracts and tests.
cli/enginecli/test_dropper.py
Outdated
| @@ -90,6 +90,10 @@ def setUpClass(cls) -> None: | |||
| cls.mintable_terminus_pool_id, cls.dropper.address, {"from": accounts[0]} | |||
| ) | |||
|
|
|||
| terminus_info_1, terminus_info_2 = cls.dropper.get_terminus_admin_info() | |||
| DropperTestCase().assertEqual(cls.terminus.address, terminus_info_1) | |||
There was a problem hiding this comment.
@ShivaLaguna : Can you make a separate test for this? I don't like to put tests in the setup code.
contracts/Dropper/DropperFacet.sol
Outdated
| @@ -111,6 +111,11 @@ contract DropperFacet is | |||
| ds.TerminusAdminPoolID = terminusAdminPoolID; | |||
| } | |||
|
|
|||
| function getTerminusAdminInfo() external view returns (address, uint) { | |||
There was a problem hiding this comment.
@ShivaLaguna : Can you rename this function to be consistent with our other contracts? See for example Garden of Forking Paths: https://github.com/bugout-dev/engine/blob/main/contracts/mechanics/garden-of-forking-paths/GardenOfForkingPaths.sol#L204
There was a problem hiding this comment.
Name should be adminTerminusInfo.
Changes
getTerminusAdminInfoto DropperFacetDropperFacet.pygetTerminusAdminInfoHow to test these changes?
cd cli./test.sh enginecli/test_dropper.pyRelated issues