Skip to content

Commit

Permalink
some review fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
TonioMacaronio committed Sep 5, 2019
1 parent 18758d7 commit 3cee023
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 24 deletions.
6 changes: 3 additions & 3 deletions contracts/contracts/Bytes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ library Bytes {
// Returns the newly created 'bytes memory'
// The returned bytes will be of length 'len'.
function toBytesFromBytes32(bytes32 self, uint8 len) internal pure returns (bytes memory bts) {
require(len <= 32, "wrong bytes length");
require(len <= 32, "wrong bytes length from 32");
bts = new bytes(len);
// Even though the bytes will allocate a full word, we don't want
// any potential garbage bytes in there.
Expand All @@ -42,7 +42,7 @@ library Bytes {
pure
returns (address addr)
{
require(self.length >= 20, "wrong bytes length");
require(self.length >= 20, "wrong bytes length to address");

assembly {
addr := div(mload(add(add(self, 0x20), 0)), 0x1000000000000000000000000)
Expand All @@ -56,7 +56,7 @@ library Bytes {
pure
returns (uint128)
{
require(self.length >= 16, "wrong bytes length");
require(self.length >= 16, "wrong bytes length to 128");
uint128 tempUint;

assembly {
Expand Down
30 changes: 16 additions & 14 deletions contracts/contracts/Franklin.sol
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ contract Franklin {
// Params:
// - _count - number of requests to remove
function removePriorityRequests(uint32 _count) internal {
require(_count <= totalPriorityRequests, "Count of removed requests is higher than their count");
require(_count <= totalPriorityRequests, "large count for remove priority");

for (uint32 i = firstPriorityRequestId; i < firstPriorityRequestId + _count; i++) {
delete priorityRequestsParams[i];
Expand All @@ -273,7 +273,7 @@ contract Franklin {
// - _opType - operation type
// - _anyRequestsCount - count of requests where to look certain requests for
function removePriorityRequestsWithType(OpType _opType, uint32 _anyRequestsCount) internal {
require(_anyRequestsCount <= totalPriorityRequests, "Count of removed requests is higher than their count");
require(_anyRequestsCount <= totalPriorityRequests, "large count for remove priority with type");

uint32 removingPriorityCount = 0;
for (uint32 i = firstPriorityRequestId; i < firstPriorityRequestId + _anyRequestsCount; i++) {
Expand All @@ -290,7 +290,7 @@ contract Franklin {
// Params:
// - _anyRequestsCount - count of requests where to look deposit requests for
function accrueBalancesFromDeposits(uint32 _anyRequestsCount) internal {
require(_anyRequestsCount <= totalPriorityRequests, "Count of removed requests is higher than their count");
require(_anyRequestsCount <= totalPriorityRequests, "large count for accrue deposit balances");

for (uint32 i = firstPriorityRequestId; i < firstPriorityRequestId + _anyRequestsCount; i++) {
if (priorityRequestsParams[i].opType == OpType.Deposit) {
Expand Down Expand Up @@ -344,7 +344,7 @@ contract Franklin {
// Params:
// - _franklinAddr - receiver
function depositETH(bytes calldata _franklinAddr) external payable {
require(msg.value <= MAX_VALUE, "sorry Joe");
require(msg.value <= MAX_VALUE, "deposit value over max");
registerDeposit(ValidatedTokenId(0), uint128(msg.value), _franklinAddr);
}

Expand All @@ -368,7 +368,7 @@ contract Franklin {
) external {
require(
IERC20(_token).transferFrom(msg.sender, address(this), _amount),
"transfer failed"
"token transfer failed deposit"
);
ValidatedTokenId memory tokenId = ValidatedTokenId({
id: governance.validateERC20Token(_token)
Expand All @@ -387,19 +387,17 @@ contract Franklin {
registerWithdrawal(tokenId, _amount);
require(
IERC20(_token).transfer(msg.sender, _amount),
"transfer failed"
"token transfer failed withdraw"
);
}

// Register full exit request
// Params:
// - _franklinAddr - sender
// - _eth_addr - receiver
// - _token - token address
// - _signature - user signature
function registerFullExit(
bytes calldata _franklinAddr,
address _eth_addr,
address _token,
bytes calldata signature
) external {
Expand All @@ -409,7 +407,7 @@ contract Franklin {
});
// Priority Queue request
bytes memory pubData = _franklinAddr; // franklin address
pubData = Bytes.concat(pubData, Bytes.toBytesFromAddress(_eth_addr)); // eth address
pubData = Bytes.concat(pubData, Bytes.toBytesFromAddress(msg.sender)); // eth address
pubData = Bytes.concat(pubData, Bytes.toBytesFromUInt16(tokenId.id)); // token id
pubData = Bytes.concat(pubData, signature); // signature
addPriorityRequest(OpType.FullExit, pubData);
Expand Down Expand Up @@ -450,7 +448,7 @@ contract Franklin {
requireActive();
require(
balancesToWithdraw[msg.sender][_token.id] >= _amount,
"insufficient balance"
"insufficient balance withdraw"
);
balancesToWithdraw[msg.sender][_token.id] -= _amount;
emit OnchainWithdrawal(
Expand All @@ -475,7 +473,9 @@ contract Franklin {
bytes calldata _publicData
) external {
requireActive();
governance.requireValidator(msg.sender);
require(
governance.isValidator(msg.sender),
"not a validator in commit");
require(
_blockNumber == totalBlocksCommited + 1,
"only commit next block"
Expand All @@ -487,7 +487,7 @@ contract Franklin {
);

// Enter exodus mode if needed
require(!triggerExodusIfNeeded(), "Entered exodus mode");
require(!triggerExodusIfNeeded(), "entered exodus mode");

(uint64 startId, uint64 totalProcessed, uint32 priorityOperations) = commitOnchainOps(_publicData);

Expand Down Expand Up @@ -700,7 +700,7 @@ contract Franklin {
return (FULL_EXIT_LENGTH, 1, 1);
}

require(false, "unsupported op");
revert("unsupported op");
}

// MARK: - Block verification
Expand All @@ -714,7 +714,9 @@ contract Franklin {
external
{
requireActive();
governance.requireValidator(msg.sender);
require(
governance.isValidator(msg.sender),
"not a validator in verify");
require(
_blockNumber == totalBlocksVerified + 1,
"only verify next block"
Expand Down
4 changes: 2 additions & 2 deletions contracts/contracts/Governance.sol
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ contract Governance {
}

// Check if sender is validator
function requireValidator(address sender) external view {
require(validators[sender], "only by validator");
function isValidator(address sender) external view returns (bool) {
return validators[sender];
}

// Check if token is known
Expand Down
10 changes: 5 additions & 5 deletions docs/contract.md
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,16 @@ When corresponding transactions are found in the commited block, their count mus

If the block is reverted, the funds held by **Deposit priority requests** from this block are acrued to the owners' **root-chain balances** to make them possible to withdraw. And this **Deposit priority requests** will be removed from mapping.

A certain value of the selected token will be withdrawn from the _user's_ account immediately when he send a request, as payment for the _validator’s_ work to include these transactions in the block. One transaction fee is calculated as follows:
`fee = 3 * gas * mediumFee`, where
- `gas` - the gas cost of all related operations for the exit
- `mediumFee` - current average fee in the network.

### **Validators'** responsibility

**Validators** MUST subscribe for `NewPriorityRequest` events in the RIGHT order to include priority transactions in some upcoming blocks.
The need for _Validators_ to include these transactions in blocks as soon as possible is dedicated by the increasing probability of entering the **Exodus Mode** (described below).

A certain value of the selected token will be withdrawn from the _user's_ account, as payment for the _validator’s_ work to include these transactions in the block. One transaction fee is calculated as follows:
`fee = 3 * gas * mediumFee`, where
- `gas` - the gas cost of all related operations for the exit
- `mediumFee` - current average fee in the network.

### Exodus mode

If the **Requests Queue** is being processed too slow, it will trigger the **Exodus mode** in **Franklin** contract. This moment is determined by the first (oldest) **priority request** with oldest `expirationBlock` value . If `current ethereum block number >= oldest expiration block number` the **Exodus Mode** will be entered.
Expand Down

0 comments on commit 3cee023

Please sign in to comment.