First Draft of proxy operations on contract#9
Draft
deluca-mike wants to merge 7 commits into
Draft
Conversation
gtabmx
force-pushed
the
feature/proxyContractLogic
branch
2 times, most recently
from
714b78d to
b2298cd
Compare
fish-sammy
reviewed
Oct 24, 2019
fish-sammy
reviewed
Oct 24, 2019
fish-sammy
reviewed
Oct 24, 2019
fish-sammy
reviewed
Oct 24, 2019
fish-sammy
reviewed
Oct 24, 2019
| function getAddressFromSig(bytes32 arg, address relayer, uint256 maxGasPrice, uint256 premium, uint256 nonce, uint8 v, bytes32 r, bytes32 s) internal returns(address) { | ||
| bytes data = abi.encodePacked(arg, relayer, maxGasPrice, premium, nonce); | ||
| bytes32 hash = sha3(data); | ||
| return ecrecover(hash, v, r, s); |
There was a problem hiding this comment.
Suggested change
| return ecrecover(hash, v, r, s); | |
| address accountAddress = ecrecover(hash, v, r, s); | |
| asert(accountAddress != address(0)); | |
| return accountAddress; |
There was a problem hiding this comment.
ecrecover returnsaddress(0) on error
Collaborator
Author
There was a problem hiding this comment.
@sammy1991106 I wonder if that matters. If someone provides or relays an incorrect signature (which they could have checked via this pure function for free) then it will end up affecting address(0), which is relevant. The only person who gets hurt is the relayer that did not check the sig, or provided a bad sig.
fish-sammy
reviewed
Oct 24, 2019
| account.balance += msg.value; | ||
| account.unlockBlock = block.number + 7200; | ||
|
|
||
| emit DepositMade(msg.sender, msg.value, account.unlockBlock); |
There was a problem hiding this comment.
Suggested change
| emit DepositMade(msg.sender, msg.value, account.unlockBlock); | |
| uint256 unlockBlock = block.number + 7200; | |
| ... | |
| emit DepositMade(msg.sender, msg.value, unlockBlock); |
to avoid the redundent SLOAD
Collaborator
Author
There was a problem hiding this comment.
@sammy1991106 would the compiler not have made this optimization?
fish-sammy
reviewed
Oct 24, 2019
gtabmx
force-pushed
the
feature/proxyContractLogic
branch
2 times, most recently
from
0c36717 to
eda4744
Compare
- still needs to be tested
gtabmx
force-pushed
the
feature/proxyContractLogic
branch
from
eda4744 to
d0171bf
Compare
gtabmx
force-pushed
the
feature/proxyContractLogic
branch
from
d234971 to
971f114
Compare
gtabmx
force-pushed
the
feature/proxyContractLogic
branch
from
a8a41f4 to
1d26f8f
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Still need to implement Events