Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

script: distribute yield from treasury to staked locker #17

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

erza-eth
Copy link

No description provided.

@erza-eth erza-eth marked this pull request as draft May 17, 2022 17:02
@erza-eth
Copy link
Author

erza-eth commented May 31, 2022

To be honest, I'm not quite sure where this revert comes from:

>>> gov.citadel.gac.paused()
False
>>> gov.citadel.gac.hasRole(gov.citadel.gac.CONTRACT_GOVERNANCE_ROLE(), gov)
True
>>> gov.citadel.staked_citadel_locker.getRewardTokens()
()
>>> gov.citadel.staked_citadel_locker.addReward('0x4e3FBD56CD56c3e72c1403e103b45Db9da5B9D2B', '0x38724146C8dc1Aa49c3395
091cf86B789c37F52c', False)
Transaction sent: 0x21edc0475d3d0b0216cb24447fd4646ec9f7a089a24b300fc416d781de61e938
  Gas price: 0.0 gwei   Gas limit: 12000000   Nonce: 3
  IStakedCitadelLocker.addReward confirmed (reverted)   Block: 14877930   Gas used: 24171 (0.20%)

<Transaction '0x21edc0475d3d0b0216cb24447fd4646ec9f7a089a24b300fc416d781de61e938'>

See brownie test tests/citadel/test_distribute.py.

@erza-eth erza-eth self-assigned this May 31, 2022
@petrovska-petro
Copy link

To be honest, I'm not quite sure where this revert comes from:

>>> gov.citadel.gac.paused()
False
>>> gov.citadel.gac.hasRole(gov.citadel.gac.CONTRACT_GOVERNANCE_ROLE(), gov)
True
>>> gov.citadel.staked_citadel_locker.getRewardTokens()
()
>>> gov.citadel.staked_citadel_locker.addReward('0x4e3FBD56CD56c3e72c1403e103b45Db9da5B9D2B', '0x38724146C8dc1Aa49c3395
091cf86B789c37F52c', False)
Transaction sent: 0x21edc0475d3d0b0216cb24447fd4646ec9f7a089a24b300fc416d781de61e938
  Gas price: 0.0 gwei   Gas limit: 12000000   Nonce: 3
  IStakedCitadelLocker.addReward confirmed (reverted)   Block: 14877930   Gas used: 24171 (0.20%)

<Transaction '0x21edc0475d3d0b0216cb24447fd4646ec9f7a089a24b300fc416d781de61e938'>

See brownie test tests/citadel/test_distribute.py.

it seems that in this case the gac have not being initiliased, so on the test file requires also to call the following:

gov.citadel.staked_citadel_locker.__GlobalAccessControlManaged_init(gov.citadel.gac.address)

@erza-eth
Copy link
Author

erza-eth commented Jun 1, 2022

thanks, missed that part. added it to the config of the test, passing now.

@erza-eth erza-eth marked this pull request as ready for review June 1, 2022 11:24
@erza-eth erza-eth changed the title init draft; still needs work on setting the permissions in tests script: distribute yield from treasury to staked locker Jun 1, 2022
@erza-eth erza-eth linked an issue Jun 1, 2022 that may be closed by this pull request
petrovska-petro
petrovska-petro previously approved these changes Jun 1, 2022
Copy link

@petrovska-petro petrovska-petro left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@erza-eth erza-eth requested a review from petrovska-petro June 15, 2022 14:09

def distribute_yield(self, asset, mantissa):
assert mantissa > 0
asset = interface.ERC20(asset, owner=self.safe.account)
Copy link

@jalbrekt85 jalbrekt85 Jun 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo better to use Contract/safe.contract opposed to interface.ERC20 since asset is variable and might cause trouble in the future when trying to access functions outside of erc20 interface

)


def test_distribute(cvx):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

now that posting was included in the distribute_yield script breaks the test. i will say just relay treasury_vault.citadel.distribute_yield(...) for the test

Suggested change
def test_distribute(cvx):
def test_distribute(treasury_vault, cvx):
treasury_vault.citadel.distribute_yield(cvx.address, 123e18)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Distribute treasury yield to xCitadel
3 participants