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

fix: updated setRewards #474

Merged
merged 1 commit into from
Oct 20, 2021
Merged

fix: updated setRewards #474

merged 1 commit into from
Oct 20, 2021

Conversation

jhb10c
Copy link
Contributor

@jhb10c jhb10c commented Oct 14, 2021

This is update was requested in 467.

The contract BaseStrategy has the onlyRewards updated so that only governance can update the address. The corresponding tests have been updated.

@jhb10c jhb10c changed the title Updated setRewards to only allow governance and updated tests fix:Updated setRewards to only allow governance and updated tests Oct 14, 2021
@jhb10c jhb10c changed the title fix:Updated setRewards to only allow governance and updated tests fix: updated setRewards Oct 14, 2021
@pandadefi
Copy link
Contributor

Hello
Sorry little confusion on my reply, the function should allow existing address + governance to be able to setRewards

Also I can see you added a DSstore file that isn't needed

@jhb10c jhb10c force-pushed the main branch 2 times, most recently from 939d802 to d8e8bc7 Compare October 14, 2021 22:35
@jhb10c
Copy link
Contributor Author

jhb10c commented Oct 15, 2021

@pandadefi, I reread your original comment and I now understand what you mean.

I have added the modifier _onlyRewarder to require either strategist or goverance and applied this modifier to _setRewards. This has been tested. I ran the python formatter but I was not able to get the solidity lint to run.

(If there is any constructive criticism or issues, please comment. I am new to pull requests/contributing and I am trying to learn.)

@pandadefi
Copy link
Contributor

All checks have passed, one last change regarding the comment and I will merge.

@pandadefi pandadefi enabled auto-merge (squash) October 15, 2021 16:06
auto-merge was automatically disabled October 16, 2021 05:45

Head branch was pushed to by a user without write access

@pandadefi pandadefi enabled auto-merge (squash) October 20, 2021 08:11
@pandadefi pandadefi merged commit a113064 into yearn:main Oct 20, 2021
rareweasel pushed a commit to rareweasel/yearn-vaults that referenced this pull request Nov 9, 2021
pandadefi added a commit that referenced this pull request Oct 25, 2022
* refactor: reduce base strategy size (#422)

* refactor: reduce base strategy size

* fix: do not add onlyManagement

* fix: allow harvest on add strategy block (#428)

* fix: allow harvest on add strategy block

* docs: comment

Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>

Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>

* fix: updated setRewards (#474)

* feat: add deposit and withdraw events (#499)

* feat: add event to add_event_to_setLockedProfitDegradation (#506)

* fix: implement modifiers (#513)

* fix: implement modifiers

* fix: typo

* fix: formatting

* fix: test same time harvest not allowed

* fix: merge mistake

* fix: gov event

* fix: add pps protection

* chore: update solc version

* chore: add FeeReport event

* chore: update FeeReport

* chore: bump vyper version

* chore: bump brownie version

* test: fixes

* chore: bump ganache version

* style: fix format

* test: airdrop do not change pps

* test: make sure deposits are still possible

* ci: cancel same branch run

* style: fix python formating

* fix: wrong math operation

* fix: tests

* fix: add missing WithdrawFromStrategy event

* docs: fix comment

* chore: cleanup docs folder

* chore: remove not used code

* feat: prevent fork replay EIP712 signature (#3)

* chore: bump version

* fix: clone check (#5)

* fix: make sure strategy isn't in emergency mode when updating debt (#4)

* fix: change permit definition (#6)

* fix: change permit definition

* fix: tests

* chore: format

* fix: do not use external call

* fix: allow setEmergency after revoke (#7)

Co-authored-by: Steffel <2143646+steffenix@users.noreply.github.com>
Co-authored-by: El De-dog-lo <3859395+fubuloubu@users.noreply.github.com>
Co-authored-by: John Bergschneider <jhb10c@my.fsu.edu>
Co-authored-by: jmonteer <68742302+jmonteer@users.noreply.github.com>
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.

None yet

2 participants