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

Feature/pool 191 need a clearly defined way to create #41

Merged

Conversation

asselstine
Copy link
Contributor

This PR splits the Prize Strategy more cleanly from the Prize Pool.

  1. Prize Strategy now handles startAward and completeAward and the RNG request.
  2. Prize Pool merely allows the prize strategy to distribute tokens.

@linear
Copy link

linear bot commented Jun 19, 2020

SingleRandomWinnerPrizeStrategyFactory _prizeStrategyFactory
) public initializer {
require(address(_prizePoolBuilder) != address(0), "prize pool builder must be defined");
require(address(_rng) != address(0), "rng cannot be zero");
Copy link
Contributor

Choose a reason for hiding this comment

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

The require statements should be consistent, would recommend "rng must be defined"

@@ -53,10 +50,11 @@ abstract contract PeriodicPrizePool is Timelock, BaseRelayRecipient, ReentrancyG
event SponsorshipInterestMinted(address indexed operator, address indexed to, uint256 amount);
event SponsorshipInterestBurned(address indexed operator, address indexed from, uint256 amount);

PrizeStrategyInterface public override prizeStrategy;
event Awarded(address indexed operator, address token, uint256 amount, bytes data);
Copy link
Contributor

Choose a reason for hiding this comment

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

address token could be indexed here

@@ -695,7 +666,7 @@ abstract contract PeriodicPrizePool is Timelock, BaseRelayRecipient, ReentrancyG
// handle transfers of tickets, sponsorship, credits etc

// transfers of credits are ignored
if (msg.sender == address(ticket)) {
if (msg.sender == address(__ticket)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

"_msgSender()"

event PrizePoolAwardStarted(address indexed operator, address indexed prizePool, uint256 indexed rngRequestId);

RNGInterface public rng;
mapping(address => uint256) rngRequestIds;
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing access scope here

}

function completeAward() public override requireCanCompleteAward nonReentrant {
function awardPrize() external override onlyPrizeStrategy returns (uint256) {
require(_isPrizePeriodOver(), "PeriodiPrizePool/not-over");
Copy link
Contributor

Choose a reason for hiding this comment

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

type-o on "Periodic"

@asselstine asselstine merged commit 0528152 into version-3 Jun 22, 2020
@asselstine asselstine deleted the feature/pool-191-need-a-clearly-defined-way-to-create branch June 22, 2020 23:52
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