Apply suggestions from code review #1732

Co-Authored-By: Derek Pierre <derek.pierre@gmail.com>
pull/1773/head
vzotova 2020-03-03 12:16:52 +03:00
parent 18bffc15c1
commit fde134a398
6 changed files with 34 additions and 33 deletions

View File

@ -224,7 +224,7 @@ class BaseEconomics:
...
2 startBidDate - Timestamp when bidding starts
3 endBidDate - Timestamp when bidding will end
4 endBidDate - Timestamp when cancellation window will end
4 endCancellationDate - Timestamp when cancellation window will end
5 boostingRefund - Coefficient to boost refund ETH
6 stakingPeriods - Duration of tokens locking
7 minAllowedBid - Minimum allowed ETH amount for bidding

View File

@ -1637,6 +1637,15 @@ class Bidder(NucypherTokenActor):
class BiddingIsClosed(BidderError):
pass
class CancellationWindowIsOpen(BidderError):
pass
class CancellationWindowIsClosed(BidderError):
pass
class ClaimError(BidderError):
pass
@validate_checksum_address
def __init__(self,
checksum_address: str,
@ -1680,10 +1689,10 @@ class Bidder(NucypherTokenActor):
end = self.worklock_agent.end_cancellation_date
if ensure_closed and now < end:
message = message or f"Operation cannot be performed while the cancellation window is still open (closes at {end})."
raise self.BidderError(message)
raise self.CancellationWindowIsOpen(message)
elif not ensure_closed and now >= end:
message = message or f"Operation is allowed only while the cancellation window is open (closed at {end})."
raise self.BidderError(message)
raise self.CancellationWindowIsClosed(message)
#
# Transactions
@ -1704,20 +1713,15 @@ class Bidder(NucypherTokenActor):
self._ensure_cancellation_window(ensure_closed=True)
if not self.worklock_agent.is_claiming_available():
raise self.BidderError(f"Claiming is not available yet")
raise self.ClaimError(f"Claiming is not available yet")
# Ensure the claim was not already placed
if self._has_claimed:
raise self.BidderError(f"Bidder {self.checksum_address} already placed a claim.")
raise self.ClaimError(f"Bidder {self.checksum_address} already placed a claim.")
# Require an active bid
if not self.get_deposited_eth:
raise self.BidderError(f"No claims available for {self.checksum_address}")
# Ensure the claim is at least large enough for min. stake
minimum = self.economics.minimum_allowed_locked
if self.available_claim < minimum:
raise ValueError(f"Claim is too small. Claim amount must be worth at least {NU.from_nunits(minimum)})")
raise self.ClaimError(f"No claims available for {self.checksum_address}")
receipt = self.worklock_agent.claim(checksum_address=self.checksum_address)
return receipt
@ -1729,10 +1733,6 @@ class Bidder(NucypherTokenActor):
if not self.get_deposited_eth:
self.BidderError(f"No bids available for {self.checksum_address}")
# Ensure the claim was not already placed
if self._has_claimed:
raise self.BidderError(f"Bidder {self.checksum_address} already placed a claim.")
receipt = self.worklock_agent.cancel_bid(checksum_address=self.checksum_address)
return receipt
@ -1818,7 +1818,7 @@ class Bidder(NucypherTokenActor):
self._ensure_cancellation_window(ensure_closed=True)
if self.worklock_agent.bidders_checked():
raise self.BidderError(f"Check has already done")
raise self.BidderError(f"Check was already done")
whales = self.get_whales()
if whales:

View File

@ -72,7 +72,7 @@ contract WorkLock {
modifier afterCancellationWindow()
{
require(block.timestamp >= endCancellationDate,
"Operation is allowed when bidding and cancellation phases are over");
"Operation is allowed when cancellation phase is over");
_;
}
@ -193,7 +193,7 @@ contract WorkLock {
}
info.depositedETH = info.depositedETH.add(msg.value);
require(info.depositedETH >= minAllowedBid, "Bid must be more than minimum");
require(info.depositedETH >= minAllowedBid, "Bid must be at least minimum");
ethSupply = ethSupply.add(msg.value);
emit Bid(msg.sender, msg.value);
}
@ -202,7 +202,8 @@ contract WorkLock {
* @notice Cancel bid and refund deposited ETH
*/
function cancelBid() external {
require(block.timestamp < endCancellationDate, "Cancellation allowed only during bidding");
require(block.timestamp < endCancellationDate,
"Cancellation allowed only during cancellation window");
WorkInfo storage info = workInfo[msg.sender];
require(info.depositedETH > 0, "No bid to cancel");
require(!info.claimed, "Tokens are already claimed");
@ -322,7 +323,7 @@ contract WorkLock {
while (index < bidders.length && gasleft() > _gasToSaveState) {
address bidder = bidders[index];
require(workInfo[bidder].depositedETH <= maxBidFromMaxStake);
require(workInfo[bidder].depositedETH <= maxBidFromMaxStake, "Bid is greater than max allowable bid");
index++;
}

View File

@ -272,7 +272,7 @@ def refund(general_config, worklock_options, registry_options, force, hw_wallet)
@option_force
@option_hw_wallet
@click.option('--gas-limit', help="Gas limit per each verification transaction", type=click.IntRange(min=60000))
# TODO: Consider moving to administrator (nucypher-deploy)
# TODO: Consider moving to administrator (nucypher-deploy) #1758
def post_init(general_config, registry_options, worklock_options, force, hw_wallet, gas_limit):
"""Ensure correctness of bidding and enable claiming"""
emitter = _setup_emitter(general_config)

View File

@ -864,7 +864,7 @@ def paint_worklock_status(emitter, registry: BaseContractRegistry):
cancellation_end = MayaDT(worklock_agent.contract.functions.endCancellationDate().call())
bidding_duration = bidding_end - bidding_start
cancellation_duration = cancellation_end - bidding_end
cancellation_duration = cancellation_end - bidding_start
now = maya.now()
bidding_remaining = bidding_end - now if bidding_end > now else timedelta()
cancellation_remaining = cancellation_end - now if cancellation_end > now else timedelta()

View File

@ -48,12 +48,12 @@ MIN_ALLOWED_BID = to_wei(1, 'ether')
@pytest.fixture()
def worklock_factory(testerchain, token, escrow, token_economics, deploy_contract):
def deploy_worklock(supply, bidding_delay, cancellation_duration, boosting_refund):
def deploy_worklock(supply, bidding_delay, additional_time_to_cancel, boosting_refund):
now = testerchain.w3.eth.getBlock(block_identifier='latest').timestamp
start_bid_date = now + bidding_delay
end_bid_date = start_bid_date + BIDDING_DURATION
end_cancellation_date = end_bid_date + cancellation_duration
end_cancellation_date = end_bid_date + additional_time_to_cancel
staking_periods = 2 * token_economics.minimum_locked_periods
tx = escrow.functions.updateAllowableLockedTokens(token_economics.minimum_allowed_locked,
@ -107,7 +107,7 @@ def test_worklock(testerchain, token_economics, deploy_contract, token, escrow,
worklock = worklock_factory(supply=0,
bidding_delay=ONE_HOUR,
cancellation_duration=ONE_HOUR,
additional_time_to_cancel=ONE_HOUR,
boosting_refund=boosting_refund)
assert worklock.functions.startBidDate().call() == start_bid_date
@ -623,7 +623,7 @@ def test_reentrancy(testerchain, token_economics, deploy_contract, escrow, workl
max_bid = 2 * MIN_ALLOWED_BID
worklock = worklock_factory(supply=worklock_supply,
bidding_delay=0,
cancellation_duration=0,
additional_time_to_cancel=0,
boosting_refund=boosting_refund)
refund_log = worklock.events.Refund.createFilter(fromBlock='latest')
@ -716,7 +716,7 @@ def test_verifying_correctness(testerchain, token_economics, escrow, deploy_cont
worklock_supply = token_economics.maximum_allowed_locked + 1
worklock = worklock_factory(supply=worklock_supply,
bidding_delay=0,
cancellation_duration=0,
additional_time_to_cancel=0,
boosting_refund=boosting_refund)
# Bid
@ -736,7 +736,7 @@ def test_verifying_correctness(testerchain, token_economics, escrow, deploy_cont
worklock_supply = 3 * token_economics.maximum_allowed_locked
worklock = worklock_factory(supply=worklock_supply,
bidding_delay=0,
cancellation_duration=0,
additional_time_to_cancel=0,
boosting_refund=boosting_refund)
checks_log = worklock.events.BiddersChecked.createFilter(fromBlock='latest')
@ -772,7 +772,7 @@ def test_verifying_correctness(testerchain, token_economics, escrow, deploy_cont
worklock, checks_log = deploy_worklock(worklock_supply, max_allowed_bid)
worklock = worklock_factory(supply=worklock_supply,
bidding_delay=0,
cancellation_duration=0,
additional_time_to_cancel=0,
boosting_refund=boosting_refund)
checks_log = worklock.events.BiddersChecked.createFilter(fromBlock='latest')
@ -835,7 +835,7 @@ def test_force_refund(testerchain, token_economics, deploy_contract, worklock_fa
worklock_supply = len(bidders) * token_economics.maximum_allowed_locked
worklock = worklock_factory(supply=worklock_supply,
bidding_delay=0,
cancellation_duration=0,
additional_time_to_cancel=0,
boosting_refund=boosting_refund)
do_bids(testerchain, worklock, bidders, MIN_ALLOWED_BID)
@ -868,7 +868,7 @@ def test_force_refund(testerchain, token_economics, deploy_contract, worklock_fa
worklock_supply = len(bidders) * token_economics.maximum_allowed_locked
worklock = worklock_factory(supply=worklock_supply,
bidding_delay=0,
cancellation_duration=0,
additional_time_to_cancel=0,
boosting_refund=boosting_refund)
normal_bid = MIN_ALLOWED_BID
hidden_whales_bid = 2 * MIN_ALLOWED_BID
@ -1031,7 +1031,7 @@ def test_force_refund(testerchain, token_economics, deploy_contract, worklock_fa
max_bid = 2000 * MIN_ALLOWED_BID
worklock = worklock_factory(supply=worklock_supply,
bidding_delay=0,
cancellation_duration=0,
additional_time_to_cancel=0,
boosting_refund=boosting_refund)
small_bids = [random.randrange(MIN_ALLOWED_BID, int(1.5 * MIN_ALLOWED_BID)) for _ in range(10)]
@ -1068,7 +1068,7 @@ def test_force_refund(testerchain, token_economics, deploy_contract, worklock_fa
worklock_supply = 10 * token_economics.maximum_allowed_locked
worklock = worklock_factory(supply=worklock_supply,
bidding_delay=0,
cancellation_duration=0,
additional_time_to_cancel=0,
boosting_refund=boosting_refund,
max_bid=max_allowed_bid)