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

Implementation of tickets 1604, 1597 and 1365 #2368

Closed
8 changes: 6 additions & 2 deletions packages/contractkit/src/wrappers/SortedOracles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -98,9 +98,13 @@ export class SortedOraclesWrapper extends BaseWrapper<SortedOracles> {
oracleAddress
)

const reportedValue = toBigNumber(numerator.toString())
.dividedBy(toBigNumber(denominator.toString()))
.multipliedBy(await this.getInternalDenominator())

return toTransactionObject(
this.kit,
this.contract.methods.report(tokenAddress, numerator, denominator, lesserKey, greaterKey),
this.contract.methods.report(tokenAddress, reportedValue, lesserKey, greaterKey),
{ from: oracleAddress }
)
}
Expand Down Expand Up @@ -158,7 +162,7 @@ export class SortedOraclesWrapper extends BaseWrapper<SortedOracles> {
}

private async getInternalDenominator(): Promise<BigNumber> {
return toBigNumber(await this.contract.methods.DENOMINATOR().call())
return toBigNumber(await this.contract.methods.getDenominator().call())
}

private async findLesserAndGreaterKeys(
Expand Down
6 changes: 5 additions & 1 deletion packages/protocol/contracts/common/GasPriceMinimum.sol
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,11 @@ contract GasPriceMinimum is Ownable, Initializable, UsingRegistry {
uint256 rateNumerator;
uint256 rateDenominator;
(rateNumerator, rateDenominator) = sortedOracles.medianRate(tokenAddress);
return (gasPriceMinimum.mul(rateNumerator).div(rateDenominator));
FixidityLib.Fraction memory ratio = FixidityLib.newFixedFraction(
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: s/ratio/rate

rateNumerator,
rateDenominator
);
return (FixidityLib.newFixed(gasPriceMinimum)).multiply(ratio).fromFixed();
}
}

Expand Down
13 changes: 12 additions & 1 deletion packages/protocol/contracts/common/GoldToken.sol
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ contract GoldToken is Initializable, IERC20Token, ICeloToken {

event Approval(address indexed owner, address indexed spender, uint256 value);

event TotalSupplySet(uint256 totalSupply);

/**
* Only VM would be able to set the caller address to 0x0 unless someone
* really has the private key for 0x0
Expand All @@ -40,7 +42,16 @@ contract GoldToken is Initializable, IERC20Token, ICeloToken {
*/
// solhint-disable-next-line no-empty-blocks
function initialize() external initializer {
totalSupply_ = 0;
setInitialTotalSupply(0);
}

/**
* @notice Sets the token initial supply
* @param _totalSupply new token initial supply.
*/
function setInitialTotalSupply(uint256 _totalSupply) private {
totalSupply_ = _totalSupply;
emit TotalSupplySet(_totalSupply);
}

/**
Expand Down
19 changes: 19 additions & 0 deletions packages/protocol/contracts/common/MultiSig.sol
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ contract MultiSig is Initializable {
event OwnerAddition(address indexed owner);
event OwnerRemoval(address indexed owner);
event RequirementChange(uint256 required);
event RequirementSet(uint256 required);
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is this used?


/*
* Constants
Expand Down Expand Up @@ -109,6 +110,15 @@ contract MultiSig is Initializable {
initializer
validRequirement(_owners.length, _required)
{
setInitialOwners(_owners);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the idea here is to call addOwner and changeRequirement instead of creating new setters that will only be used on initialize.

And since those functions are external probably makes sense to have an _addOwner internal function that both initialize and addOwner calls. Some for the other function

setInitialRequired(_required);
}

/**
* @notice Sets the initial owners of the wallet.
* @param _owners List of initial owners.
*/
function setInitialOwners(address[] memory _owners) private {
for (uint256 i = 0; i < _owners.length; i++) {
require(
!isOwner[_owners[i]] && _owners[i] != address(0),
Expand All @@ -117,7 +127,16 @@ contract MultiSig is Initializable {
isOwner[_owners[i]] = true;
jfoutts-celo marked this conversation as resolved.
Show resolved Hide resolved
}
owners = _owners;
}

/**
* @notice Sets the required number of tx confirmations.
* @param _required The number of required tx confirmations.
*/
function setInitialRequired(uint256 _required) private {
require(_required > 0, "Required confirmations must be greater than zero");
required = _required;
emit RequirementSet(_required);
}

/// @dev Allows to add a new owner. Transaction has to be sent by wallet.
Expand Down
29 changes: 27 additions & 2 deletions packages/protocol/contracts/governance/EpochRewards.sol
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ contract EpochRewards is Ownable, Initializable, UsingPrecompiles, UsingRegistry
uint256 underspendAdjustmentFactor,
uint256 overspendAdjustmentFactor
);
event StartTimeSet(uint256 startTime);
event TargetVotingYield(uint256 targetVotingYield);

/**
* @notice Initializes critical variables.
Expand Down Expand Up @@ -101,8 +103,8 @@ contract EpochRewards is Ownable, Initializable, UsingPrecompiles, UsingRegistry
);
setTargetVotingGoldFraction(_targetVotingGoldFraction);
setTargetValidatorEpochPayment(_targetValidatorEpochPayment);
targetVotingYieldParams.target = FixidityLib.wrap(targetVotingYieldInitial);
startTime = now;
setTargetVotingYield(targetVotingYieldInitial);
setStartTime(block.timestamp);
}

/**
Expand Down Expand Up @@ -159,6 +161,29 @@ contract EpochRewards is Ownable, Initializable, UsingPrecompiles, UsingRegistry
return true;
}

/**
* @notice Sets the target voting yield for validators.
* @param _targetVotingYield The value of the target voting yield.
jfoutts-celo marked this conversation as resolved.
Show resolved Hide resolved
* @return True upon success.
*/
function setTargetVotingYield(uint256 _targetVotingYield) public onlyOwner returns (bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure for this contract, but we are changing the semantics here... since dureing the lifetime of the contract we can change the target voting yield.
I wonder if that's what we want.

@asaj @marekolszewski comments?

Copy link
Contributor

Choose a reason for hiding this comment

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

+1, the scope of this ticket was more about reducing tech debt than exposing setters for values that were previously not possible to set externally

targetVotingYieldParams.target = FixidityLib.wrap(_targetVotingYield);
emit TargetVotingYield(_targetVotingYield);
return true;
}

/**
* @notice Sets the start time.
* @param _startTime The time in unix.
* @return True upon success.
*/
function setStartTime(uint256 _startTime) public onlyOwner returns (bool) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same
@asaj @marekolszewski comments?

require(_startTime >= block.timestamp, "The start time must not be in the past");
startTime = _startTime;
emit StartTimeSet(_startTime);
return true;
}

/**
* @notice Sets the rewards multiplier parameters.
* @param max The max multiplier on target epoch rewards.
Expand Down
46 changes: 29 additions & 17 deletions packages/protocol/contracts/governance/Governance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ contract Governance is

event ExecutionStageDurationSet(uint256 executionStageDuration);

event LastDequeueSet(uint256 lastDequeue);

event ConstitutionSet(address indexed destination, bytes4 indexed functionId, uint256 threshold);

event ProposalQueued(
Expand Down Expand Up @@ -217,27 +219,37 @@ contract Governance is
);
_transferOwnership(msg.sender);
setRegistry(registryAddress);
approver = _approver;
concurrentProposals = _concurrentProposals;
minDeposit = _minDeposit;
queueExpiry = _queueExpiry;
dequeueFrequency = _dequeueFrequency;
stageDurations.approval = approvalStageDuration;
stageDurations.referendum = referendumStageDuration;
stageDurations.execution = executionStageDuration;
setApprover(_approver);
setConcurrentProposals(_concurrentProposals);
setMinDeposit(_minDeposit);
setQueueExpiry(_queueExpiry);
setDequeueFrequency(_dequeueFrequency);
setApprovalStageDuration(approvalStageDuration);
setReferendumStageDuration(referendumStageDuration);
setExecutionStageDuration(executionStageDuration);
setParticipationBaseline(participationBaseline);
setParticipationFloor(participationFloor);
setBaselineUpdateFactor(baselineUpdateFactor);
setBaselineQuorumFactor(baselineQuorumFactor);
setLastDequeue(block.timestamp);
}

/**
* @notice Updates the last dequeue timestamp.
* @param _lastDequeue The last dequeue timestamp.
*/
function setLastDequeue(uint256 _lastDequeue) public onlyOwner {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same...
@asaj @marekolszewski comments?

require(_lastDequeue >= block.timestamp, "last dequeuing time must not be in the past");
// solhint-disable-next-line not-rely-on-time
lastDequeue = now;
lastDequeue = _lastDequeue;
emit LastDequeueSet(_lastDequeue);
}

/**
* @notice Updates the address that has permission to approve proposals in the approval stage.
* @param _approver The address that has permission to approve proposals in the approval stage.
*/
function setApprover(address _approver) external onlyOwner {
function setApprover(address _approver) public onlyOwner {
require(_approver != address(0) && _approver != approver);
approver = _approver;
emit ApproverSet(_approver);
Expand All @@ -247,7 +259,7 @@ contract Governance is
* @notice Updates the number of proposals to dequeue at a time.
* @param _concurrentProposals The number of proposals to dequeue at at a time.
*/
function setConcurrentProposals(uint256 _concurrentProposals) external onlyOwner {
function setConcurrentProposals(uint256 _concurrentProposals) public onlyOwner {
require(_concurrentProposals > 0 && _concurrentProposals != concurrentProposals);
concurrentProposals = _concurrentProposals;
emit ConcurrentProposalsSet(_concurrentProposals);
Expand All @@ -257,7 +269,7 @@ contract Governance is
* @notice Updates the minimum deposit needed to make a proposal.
* @param _minDeposit The minimum Celo Gold deposit needed to make a proposal.
*/
function setMinDeposit(uint256 _minDeposit) external onlyOwner {
function setMinDeposit(uint256 _minDeposit) public onlyOwner {
require(_minDeposit != minDeposit);
minDeposit = _minDeposit;
emit MinDepositSet(_minDeposit);
Expand All @@ -267,7 +279,7 @@ contract Governance is
* @notice Updates the number of seconds before a queued proposal expires.
* @param _queueExpiry The number of seconds a proposal can stay in the queue before expiring.
*/
function setQueueExpiry(uint256 _queueExpiry) external onlyOwner {
function setQueueExpiry(uint256 _queueExpiry) public onlyOwner {
require(_queueExpiry > 0 && _queueExpiry != queueExpiry);
queueExpiry = _queueExpiry;
emit QueueExpirySet(_queueExpiry);
Expand All @@ -279,7 +291,7 @@ contract Governance is
* @param _dequeueFrequency The number of seconds before the next batch of proposals can be
* dequeued.
*/
function setDequeueFrequency(uint256 _dequeueFrequency) external onlyOwner {
function setDequeueFrequency(uint256 _dequeueFrequency) public onlyOwner {
require(_dequeueFrequency > 0 && _dequeueFrequency != dequeueFrequency);
dequeueFrequency = _dequeueFrequency;
emit DequeueFrequencySet(_dequeueFrequency);
Expand All @@ -289,7 +301,7 @@ contract Governance is
* @notice Updates the number of seconds proposals stay in the approval stage.
* @param approvalStageDuration The number of seconds proposals stay in the approval stage.
*/
function setApprovalStageDuration(uint256 approvalStageDuration) external onlyOwner {
function setApprovalStageDuration(uint256 approvalStageDuration) public onlyOwner {
require(approvalStageDuration > 0 && approvalStageDuration != stageDurations.approval);
stageDurations.approval = approvalStageDuration;
emit ApprovalStageDurationSet(approvalStageDuration);
Expand All @@ -299,7 +311,7 @@ contract Governance is
* @notice Updates the number of seconds proposals stay in the referendum stage.
* @param referendumStageDuration The number of seconds proposals stay in the referendum stage.
*/
function setReferendumStageDuration(uint256 referendumStageDuration) external onlyOwner {
function setReferendumStageDuration(uint256 referendumStageDuration) public onlyOwner {
require(referendumStageDuration > 0 && referendumStageDuration != stageDurations.referendum);
stageDurations.referendum = referendumStageDuration;
emit ReferendumStageDurationSet(referendumStageDuration);
Expand All @@ -309,7 +321,7 @@ contract Governance is
* @notice Updates the number of seconds proposals stay in the execution stage.
* @param executionStageDuration The number of seconds proposals stay in the execution stage.
*/
function setExecutionStageDuration(uint256 executionStageDuration) external onlyOwner {
function setExecutionStageDuration(uint256 executionStageDuration) public onlyOwner {
require(executionStageDuration > 0 && executionStageDuration != stageDurations.execution);
stageDurations.execution = executionStageDuration;
emit ExecutionStageDurationSet(executionStageDuration);
Expand Down
4 changes: 2 additions & 2 deletions packages/protocol/contracts/governance/LockedGold.sol
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,14 @@ contract LockedGold is ILockedGold, ReentrancyGuard, Initializable, UsingRegistr
function initialize(address registryAddress, uint256 _unlockingPeriod) external initializer {
_transferOwnership(msg.sender);
setRegistry(registryAddress);
unlockingPeriod = _unlockingPeriod;
setUnlockingPeriod(_unlockingPeriod);
}

/**
* @notice Sets the duration in seconds users must wait before withdrawing gold after unlocking.
* @param value The unlocking period in seconds.
*/
function setUnlockingPeriod(uint256 value) external onlyOwner {
function setUnlockingPeriod(uint256 value) public onlyOwner {
require(value != unlockingPeriod);
unlockingPeriod = value;
emit UnlockingPeriodSet(value);
Expand Down
11 changes: 6 additions & 5 deletions packages/protocol/contracts/stability/Exchange.sol
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import "./interfaces/IExchange.sol";
import "./interfaces/ISortedOracles.sol";
import "./interfaces/IReserve.sol";
import "./interfaces/IStableToken.sol";
import "../common/FractionUtil.sol";
import "../common/Initializable.sol";
import "../common/FixidityLib.sol";
import "../baklava/Freezable.sol";
Expand All @@ -19,7 +18,6 @@ import "../common/UsingRegistry.sol";
*/
contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, ReentrancyGuard, Freezable {
using SafeMath for uint256;
using FractionUtil for FractionUtil.Fraction;
using FixidityLib for FixidityLib.Fraction;

event Exchanged(address indexed exchanger, uint256 sellAmount, uint256 buyAmount, bool soldGold);
Expand Down Expand Up @@ -295,7 +293,9 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc

function getUpdatedBuckets() private view returns (uint256, uint256) {
uint256 updatedGoldBucket = getUpdatedGoldBucket();
uint256 updatedStableBucket = getOracleExchangeRate().mul(updatedGoldBucket);
uint256 updatedStableBucket = getOracleExchangeRate()
.multiply(FixidityLib.newFixed(updatedGoldBucket))
.fromFixed();

return (updatedGoldBucket, updatedStableBucket);
}
Expand Down Expand Up @@ -350,13 +350,14 @@ contract Exchange is IExchange, Initializable, Ownable, UsingRegistry, Reentranc
return timePassed && enoughReports && medianReportRecent;
}

function getOracleExchangeRate() private view returns (FractionUtil.Fraction memory) {
function getOracleExchangeRate() private view returns (FixidityLib.Fraction memory) {
uint256 rateNumerator;
uint256 rateDenominator;
(rateNumerator, rateDenominator) = ISortedOracles(
registry.getAddressForOrDie(SORTED_ORACLES_REGISTRY_ID)
)
.medianRate(stable);
return FractionUtil.Fraction(rateNumerator, rateDenominator);

return FixidityLib.newFixedFraction(rateNumerator, rateDenominator);
}
}
4 changes: 2 additions & 2 deletions packages/protocol/contracts/stability/Reserve.sol
Original file line number Diff line number Diff line change
Expand Up @@ -48,14 +48,14 @@ contract Reserve is IReserve, Ownable, Initializable, UsingRegistry, ReentrancyG
{
_transferOwnership(msg.sender);
setRegistry(registryAddress);
tobinTaxStalenessThreshold = _tobinTaxStalenessThreshold;
setTobinTaxStalenessThreshold(_tobinTaxStalenessThreshold);
}

/**
* @notice Sets the number of seconds to cache the tobin tax value for.
* @param value The number of seconds to cache the tobin tax value for.
*/
function setTobinTaxStalenessThreshold(uint256 value) external onlyOwner {
function setTobinTaxStalenessThreshold(uint256 value) public onlyOwner {
require(value > 0, "value was zero");
tobinTaxStalenessThreshold = value;
emit TobinTaxStalenessThresholdSet(value);
Expand Down
Loading