Skip to content

Commit

Permalink
OG-408 forwarder validUntil (#582) + truffle solc compiler workaround
Browse files Browse the repository at this point in the history
* OG-408: forwarder validUntil

make sure forwrader signed requests don't last forever, by adding
"validUntil" block number.

client defaults to 6000 blocks (~1 day).

relayer validates request is at least 3000 blocks into the future.

* fix truffle's "missing compiler" bug
  • Loading branch information
drortirosh authored Feb 8, 2021
1 parent 38e5732 commit 5d562f8
Show file tree
Hide file tree
Showing 23 changed files with 145 additions and 50 deletions.
4 changes: 4 additions & 0 deletions .circleci/config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ jobs: # a collection of steps
steps: # a collection of executable commands
- checkout # special step to check out source code to working directory

- run:
name: download missing solc for truffle
command: ./scripts/download-solc

- restore_cache: # special step to restore the dependency cache
key: dependency-cache-{{ checksum "package.json" }}-yarn2
- run:
Expand Down
13 changes: 6 additions & 7 deletions contracts/forwarder/Forwarder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import "./IForwarder.sol";
contract Forwarder is IForwarder {
using ECDSA for bytes32;

string public constant GENERIC_PARAMS = "address from,address to,uint256 value,uint256 gas,uint256 nonce,bytes data";
string public constant GENERIC_PARAMS = "address from,address to,uint256 value,uint256 gas,uint256 nonce,bytes data,uint256 validUntil";

string public constant EIP712_DOMAIN_TYPE = "EIP712Domain(string name,string version,uint256 chainId,address verifyingContract)";

Expand Down Expand Up @@ -60,12 +60,15 @@ contract Forwarder is IForwarder {

bytes memory callData = abi.encodePacked(req.data, req.from);
require( gasleft()*63/64 >= req.gas, "FWD: insufficient gas" );
require(req.validUntil == 0 || req.validUntil > block.number, "FWD: request expired");

// solhint-disable-next-line avoid-low-level-calls
(success,ret) = req.to.call{gas : req.gas, value : req.value}(callData);
if ( address(this).balance>0 ) {
//can't fail: req.from signed (off-chain) the request, so it must be an EOA...
payable(req.from).transfer(address(this).balance);
}

return (success,ret);
}

Expand Down Expand Up @@ -114,11 +117,6 @@ contract Forwarder is IForwarder {
emit RequestTypeRegistered(requestTypehash, requestType);
}

event DomainRegistered(bytes32 indexed domainSeparator, bytes domainValue);

event RequestTypeRegistered(bytes32 indexed typeHash, string typeStr);


function _verifySig(
ForwardRequest calldata req,
bytes32 domainSeparator,
Expand Down Expand Up @@ -155,7 +153,8 @@ contract Forwarder is IForwarder {
req.value,
req.gas,
req.nonce,
keccak256(req.data)
keccak256(req.data),
req.validUntil
),
suffixData
);
Expand Down
5 changes: 5 additions & 0 deletions contracts/forwarder/IForwarder.sol
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,13 @@ interface IForwarder {
uint256 gas;
uint256 nonce;
bytes data;
uint64 validUntil;
}

event DomainRegistered(bytes32 indexed domainSeparator, bytes domainValue);

event RequestTypeRegistered(bytes32 indexed typeHash, string typeStr);

function getNonce(address from)
external view
returns(uint256);
Expand Down
5 changes: 3 additions & 2 deletions contracts/utils/GsnEip712Library.sol
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ library GsnEip712Library {
uint256 private constant MAX_RETURN_SIZE = 1024;

//copied from Forwarder (can't reference string constants even from another library)
string public constant GENERIC_PARAMS = "address from,address to,uint256 value,uint256 gas,uint256 nonce,bytes data";
string public constant GENERIC_PARAMS = "address from,address to,uint256 value,uint256 gas,uint256 nonce,bytes data,uint256 validUntil";

bytes public constant RELAYDATA_TYPE = "RelayData(uint256 gasPrice,uint256 pctRelayFee,uint256 baseRelayFee,address relayWorker,address paymaster,address forwarder,bytes paymasterData,uint256 clientId)";

Expand Down Expand Up @@ -56,7 +56,8 @@ library GsnEip712Library {
req.request.value,
req.request.gas,
req.request.nonce,
req.request.data
req.request.data,
req.request.validUntil
);
suffixData = abi.encode(
hashRelayData(req.relayData));
Expand Down
9 changes: 9 additions & 0 deletions scripts/download-solc
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#!/bin/bash -xe
FOLDER=~/.config/truffle/compilers/node_modules/
SOLC=soljson-v0.7.6+commit.7338295f.js
mkdir -p $FOLDER

COMPILER=$FOLDER/$SOLC
curl -sL https://github.com/ethereum/solidity/releases/download/v0.7.6/soljson.js > $COMPILER
md5sum $COMPILER | grep bd086ddf0f3585a0373c086e001982c3

1 change: 1 addition & 0 deletions src/common/EIP712/ForwardRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,4 +8,5 @@ export default interface ForwardRequest {
value: IntString
nonce: IntString
gas: IntString
validUntil: IntString
}
3 changes: 2 additions & 1 deletion src/common/EIP712/TypedRequestData.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ const ForwardRequestType = [
{ name: 'value', type: 'uint256' },
{ name: 'gas', type: 'uint256' },
{ name: 'nonce', type: 'uint256' },
{ name: 'data', type: 'bytes' }
{ name: 'data', type: 'bytes' },
{ name: 'validUntil', type: 'uint256' }
]

const RelayRequestType = [
Expand Down
2 changes: 1 addition & 1 deletion src/common/Environments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ interface Environment {

export const defaultRelayHubConfiguration: RelayHubConfiguration = {
gasOverhead: 33346,
postOverhead: 13016,
postOverhead: 13302,
gasReserve: 100000,
maxWorkerCount: 10,
minimumStake: 1e18.toString(),
Expand Down
3 changes: 2 additions & 1 deletion src/common/types/RelayTransactionRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ export const RelayTransactionRequestShape = {
data: ow.string,
value: ow.string,
nonce: ow.string,
gas: ow.string
gas: ow.string,
validUntil: ow.string
},
relayData: {
gasPrice: ow.string,
Expand Down
10 changes: 9 additions & 1 deletion src/relayclient/RelayClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,9 @@ import {
GsnValidateRequestEvent
} from './GsnEvents'

// forwarder requests are signed with expiration time.
const REQUEST_VALID_BLOCKS = 6000 // roughly a day

// generate "approvalData" and "paymasterData" for a request.
// both are bytes arrays. paymasterData is part of the client request.
// approvalData is created after request is filled and signed.
Expand Down Expand Up @@ -306,6 +309,10 @@ export class RelayClient {
throw new Error('Contract addresses are not initialized!')
}

// valid that many blocks into the future.
const validUntilPromise = this.dependencies.contractInteractor.getBlockNumber()
.then((num: number) => (num + REQUEST_VALID_BLOCKS).toString())

const senderNonce = await this.dependencies.contractInteractor.getSenderNonce(gsnTransactionDetails.from, forwarder)
const relayWorker = relayInfo.pingResponse.relayWorkerAddress
const gasPriceHex = gsnTransactionDetails.gasPrice
Expand All @@ -329,7 +336,8 @@ export class RelayClient {
from: gsnTransactionDetails.from,
value: value,
nonce: senderNonce,
gas: gasLimit
gas: gasLimit,
validUntil: await validUntilPromise
},
relayData: {
pctRelayFee: relayInfo.relayInfo.pctRelayFee,
Expand Down
13 changes: 10 additions & 3 deletions src/relayserver/RelayServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ export class RelayServer extends EventEmitter {
}
}

validateInput (req: RelayTransactionRequest): void {
validateInput (req: RelayTransactionRequest, currentBlockNumber: number): void {
// Check that the relayHub is the correct one
if (req.metadata.relayHubAddress !== this.relayHubContract.address) {
throw new Error(
Expand All @@ -144,6 +144,13 @@ export class RelayServer extends EventEmitter {
throw new Error(
`Unacceptable gasPrice: relayServer's gasPrice:${this.gasPrice} request's gasPrice: ${req.relayRequest.relayData.gasPrice}`)
}

// validate the validUntil is not too close
const expiredInBlocks = parseInt(req.relayRequest.request.validUntil) - currentBlockNumber
if (expiredInBlocks < this.config.requestMinValidBlocks) {
throw new Error(
`Request expired (or too close): expired in ${expiredInBlocks} blocks, we expect it to be valid for ${this.config.requestMinValidBlocks}`)
}
}

validateFees (req: RelayTransactionRequest): void {
Expand Down Expand Up @@ -279,7 +286,8 @@ returnValue | ${viewRelayCallRet.returnValue}
this.logger.error('Alerted state: slowing down traffic')
await sleep(randomInRange(this.config.minAlertedDelayMS, this.config.maxAlertedDelayMS))
}
this.validateInput(req)
const currentBlock = await this.contractInteractor.getBlockNumber()
this.validateInput(req, currentBlock)
this.validateFees(req)
await this.validateMaxNonce(req.metadata.relayMaxNonce)

Expand All @@ -298,7 +306,6 @@ returnValue | ${viewRelayCallRet.returnValue}

const method = this.relayHubContract.contract.methods.relayCall(
acceptanceBudget, req.relayRequest, req.metadata.signature, req.metadata.approvalData, maxPossibleGas)
const currentBlock = await this.contractInteractor.getBlockNumber()
const details: SendTransactionDetails =
{
signer: this.workerAddress,
Expand Down
4 changes: 4 additions & 0 deletions src/relayserver/ServerConfigParams.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ export interface ServerConfigParams {
retryGasPriceFactor: number
maxGasPrice: string
defaultGasLimit: number
requestMinValidBlocks: number

runPenalizer: boolean
runPaymasterReputations: boolean
Expand Down Expand Up @@ -119,6 +120,8 @@ const serverDefaultConfiguration: ServerConfigParams = {
retryGasPriceFactor: 1.2,
defaultGasLimit: 500000,
maxGasPrice: 100e9.toString(),

requestMinValidBlocks: 3000, // roughly 12 hours (half client's default of 6000 blocks
runPaymasterReputations: true
}

Expand Down Expand Up @@ -160,6 +163,7 @@ const ConfigParamsTypes = {
managerTargetBalance: 'number',
minHubWithdrawalBalance: 'number',
defaultGasLimit: 'number',
requestMinValidBlocks: 'number',

trustedPaymasters: 'list',

Expand Down
7 changes: 4 additions & 3 deletions test/PaymasterCommitment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ Promise<{ req: RelayRequest, sig: PrefixedHexString }> {
// - PM always pay for non-reverted TXs (either high or low gas use)
// - if preRelayedCall reverts: PM always pay (=as long as commitment>preRelayedCallGasLimit)
// - standard forwarder reverts: PM always pay (since commitment > gas of (preRelayedCall,forwarder))
// - nonstandard forwardeR: PM pays above commitment
// - nonstandard forwarder: PM pays above commitment
// - trusted recipient: PM pays above commitment.
contract('Paymaster Commitment', function ([_, relayOwner, relayManager, relayWorker, senderAddress, other]) { // eslint-disable-line no-unused-vars
const RelayCallStatusCodes = {
Expand Down Expand Up @@ -146,7 +146,8 @@ contract('Paymaster Commitment', function ([_, relayOwner, relayManager, relayWo
from: senderAddress,
nonce: senderNonce,
value: '0',
gas: gasLimit
gas: gasLimit,
validUntil: '0'
},
relayData: {
pctRelayFee,
Expand Down Expand Up @@ -193,7 +194,7 @@ contract('Paymaster Commitment', function ([_, relayOwner, relayManager, relayWo
it('paymaster should not change its acceptanceBudget before transaction', async () => {
// the protocol of the relay to perform a view function of relayCall(), and then
// issue it on-chain.
// this test comes to verify the paymaster didn't chagne its acceptanceBalance between these
// this test comes to verify the paymaster didn't change its acceptanceBalance between these
// calls to a higher value.
// it is assumed that the relay already made the view function and validated the acceptanceBalance to
// be small, and now making a 2nd call on-chain, but with the acceptanceBalance as parameter.
Expand Down
3 changes: 2 additions & 1 deletion test/RelayHub.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,7 +188,8 @@ contract('RelayHub', function ([_, relayOwner, relayManager, relayWorker, sender
from: senderAddress,
nonce: senderNonce,
value: '0',
gas: gasLimit
gas: gasLimit,
validUntil: '0'
},
relayData: {
pctRelayFee,
Expand Down
11 changes: 7 additions & 4 deletions test/RelayHubGasCalculations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ contract('RelayHub gas calculations', function ([_, relayOwner, relayWorker, rel

const senderNonce = new BN('0')
const magicNumbers = {
pre: 5451,
pre: 5429,
post: 1639
}

Expand Down Expand Up @@ -87,7 +87,8 @@ contract('RelayHub gas calculations', function ([_, relayOwner, relayWorker, rel
from: senderAddress,
nonce: senderNonce.toString(),
value: '0',
gas: gasLimit.toString()
gas: gasLimit.toString(),
validUntil: '0'
},
relayData: {
baseRelayFee: baseFee.toString(),
Expand Down Expand Up @@ -298,7 +299,8 @@ contract('RelayHub gas calculations', function ([_, relayOwner, relayWorker, rel
from: senderAddress,
nonce: senderNonce,
value: '0',
gas: gasLimit.toString()
gas: gasLimit.toString(),
validUntil: '0'
},
relayData: {
baseRelayFee: '0',
Expand Down Expand Up @@ -364,7 +366,8 @@ contract('RelayHub gas calculations', function ([_, relayOwner, relayWorker, rel
from: senderAddress,
nonce: senderNonce,
value: '0',
gas: gasLimit.toString()
gas: gasLimit.toString(),
validUntil: '0'
},
relayData: {
baseRelayFee,
Expand Down
9 changes: 6 additions & 3 deletions test/RelayHubPenalizations.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,8 @@ contract('RelayHub Penalizations', function ([_, relayOwner, relayWorker, otherR
from: sender,
nonce: '0',
value: '0',
gas: gasLimit.toString()
gas: gasLimit.toString(),
validUntil: '0'
},
relayData: {
gasPrice: gasPrice.toString(),
Expand Down Expand Up @@ -354,7 +355,8 @@ contract('RelayHub Penalizations', function ([_, relayOwner, relayWorker, otherR
from: sender,
nonce: senderNonce.toString(),
value: '0',
gas: gasLimit.toString()
gas: gasLimit.toString(),
validUntil: '0'
},
relayData: {
gasPrice: gasPrice.toString(),
Expand Down Expand Up @@ -458,7 +460,8 @@ contract('RelayHub Penalizations', function ([_, relayOwner, relayWorker, otherR
from: encodedCallArgs.sender,
nonce: encodedCallArgs.nonce.toString(),
value: '0',
gas: encodedCallArgs.gasLimit.toString()
gas: encodedCallArgs.gasLimit.toString(),
validUntil: '0'
},
relayData: {
baseRelayFee: encodedCallArgs.baseFee.toString(),
Expand Down
3 changes: 2 additions & 1 deletion test/TestBatchForwarder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,8 @@ contract('BatchForwarder', ([from, relayManager, relayWorker, relayOwner]) => {
from,
nonce: '1',
value: '0',
gas: 1e6.toString()
gas: 1e6.toString(),
validUntil: '0'
},
relayData: {
pctRelayFee: '1',
Expand Down
3 changes: 2 additions & 1 deletion test/Utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@ contract('Utils', function (accounts) {
from: senderAddress,
nonce: senderNonce,
value: '0',
gas: gasLimit
gas: gasLimit,
validUntil: '0'
},
relayData: {
gasPrice,
Expand Down
6 changes: 4 additions & 2 deletions test/common/ContractInteractor.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,8 @@ contract('ContractInteractor', function (accounts) {
from: constants.ZERO_ADDRESS,
nonce: '1',
value: '0',
gas: '50000'
gas: '50000',
validUntil: '0'
},
relayData: {
gasPrice: '1',
Expand Down Expand Up @@ -113,7 +114,8 @@ contract('ContractInteractor', function (accounts) {
from: addr(2),
nonce: '1',
value: '0',
gas: '50000'
gas: '50000',
validUntil: '0'
},
relayData: {
gasPrice: '1',
Expand Down
Loading

0 comments on commit 5d562f8

Please sign in to comment.