Giter VIP home page Giter VIP logo

gsn's People

Contributors

akkien avatar alcedo avatar alexhandru avatar amaurer avatar cedoor avatar chaitanyapotti avatar dadaphl avatar davidyuk avatar dependabot[bot] avatar dev1644 avatar dmihal avatar drortirosh avatar forshtat avatar frangio avatar lirazsiri avatar nervehammer avatar neutiyoo avatar nventuro avatar pcowgill avatar qbzzt avatar qq7 avatar rrecuero avatar shahafn avatar spalladino avatar sshelton76 avatar tzann avatar yoav-tabookey avatar

Stargazers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

Watchers

 avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar  avatar

gsn's Issues

Handle timeout for http request

When the client makes an http request (to a relay), there's no protection against unreachable host (which can take >30 seconds)
The client should enable read timeout of the underlying XmlHttpRequest to abort http request after 20 seconds.

Protect from reverts in 'accept_relayed_call', 'post_relay_call'

Our 'executeWithGas' function uses low-level 'call' command.
Currently, only an actual relayed call is executed with 'executeWithGas'. This means that a relay will not be compensated if 'post_relayed_call' (or 'accept_relayed_call') revert unexpectedly, as those reverts will bubble up.
Should wrap all calls into the contract, including accept_relayed_call() and post_relayed_call()

RelayProvider does not support event subscriptions

Currently, the RelayProvider only support the send and sendAsync methods.
As a result, the web3 instance that uses it can't use event subscription.

suggested fix:
When initialized, the RelayProvider should scan its wrapped provider, and proxy all its methods.
(note: Probably most methods can be forwarded as-is. There is still a chance, though, that some will require special handling)

Provider should detect a mismatch between 'from' field and the signature

Consider the following scenario:
While adding support for an 'ephemeral keys' to the app, the developer adds these lines:

 let relayclient = provider.relayClient;
  let keypair = relayclient.newEphemeralKeypair();
  relayclient.useKeypairForSigning(keypair);

now, in a different place in code, when calling some solidity method, he does:

const tx = await this.instance.postMessage(this.state.value).send({from: this.accounts[0]});

This works fine when using Metamask, because Metamask provides a single account and handles signing for you. But in "Ephemeral Keypair" mode this will fail with a wierd error, because the transaction will be signed by an "ephemeral key" but given to a Relay as if it was coming from accounts[0], causing "can_relay() view function returned error code=1"

Client: hook strategy

Client should get a strategy() function, which decides

  • which relays to use
  • in which order.

Naive implementation is

  • filter based on minimum stake time, value
  • sort by txfee

Rebroadcast all unconfirmed txs after bumping fee

Currently, we schedule a task to resend the first unconfirmed tx and bump its fee if needed, so the relay can keep on serving without having its nonce stuck.
The problem: if the relay serves users in a higher tempo than task is being called, and all of the unconfirmed txs need their fees bumped, then the list of unconfirmed tx will grow and inevitably some of them would be dropped from the mempool, only to be sent again once the scheduled task gets to them, one by one.
To optimize it, we can simply bump fee of the first unconfirmed tx, and rebroadcast all the following ones.

No default txfee

When using the RelayProvider, we MUST specify "txfee".
Without it, the client will create a transaction with ZERO "txfee" and almost definitely will fail to find a relay.

Steps to reproduce:

  1. Checkout https://github.com/tabookey/metacoin, branch "with-relay"
  2. remove 'txfee' value
  3. npm run dev

Expected:
The transaction completes successfully

Actual:
The transaction fails, having 0 as "txfee"

Accurate recipient charging is not feasible

There's an issue with assuming that the "gas overhead" of relaying is constant. This gas overhead is what can't be measured on-chain by using gasleft in relayCall, and includes stuff like the Solidity function dispatch code and the base transaction fee.

The issue we've identified is that the gas overhead includes the transaction data fee, which is 68 gas per non zero byte and 4 gas per zero byte, which is dynamic, highly variable, and can't be cheaply calculated on chain.

As a workaround we can use a constant value as overhead, representing the average case, economically forcing relays to check off-chain that the specific encodedData is not unreasonably high. It could simply reject such requests, or only accept them if the transaction fee is large enough. Relays with higher transaction fees may be more lax in this requirement.

RelayHttpServer failed to get stake, stuck on waiting for stake

I already deployed a RelayHub for goerli network at address 0xfe7ec2db12986896e1de65eac210a7015eb816cb. Added stake and register the relay node, also relay was added and the event was emitted for the same.
I build a RelayHttpServer using make build-server and ran it using ./build/server/bin/RelayHttpServer -RelayHubAddress 0xfe7ec2db12986896e1de65eac210a7015eb816cb -EthereumNodeUrl "https://goerli.blockscout.com". But it is stuck and still waiting for stake with the following errors:

nervehammer@relayserver:~/tabookey-gasless$ ./build/server/bin/RelayHttpServer -RelayHubAddress 0xfe7ec2db12986896e1de65eac210a7015eb816cb -EthereumNodeUrl "https://goerli.blockscout.com"
2019/03/25 17:01:41 RelayHttpServer.go:40: RelayHttpServer starting. version: 0.3.5
2019/03/25 17:01:41 RelayHttpServer.go:239: Using default published port given in url:
2019/03/25 17:01:41 RelayHttpServer.go:255: Using RelayHub address: 0xfe7EC2db12986896E1de65eAc210A7015eB816cb
2019/03/25 17:01:41 RelayHttpServer.go:256: Using workdir: /home/nervehammer/tabookey-gasless/build/server
2019/03/25 17:01:41 RelayHttpServer.go:257: shortsleep? false
2019/03/25 17:01:41 RelayHttpServer.go:264: Constructing relay server in url http://localhost:8090
2019/03/25 17:01:41 utils.go:40: ks accounts len 1
2019/03/25 17:01:41 utils.go:66: key extracted. addr: 0xC27129680f17787061A03CEfEF37E52ef4a0Cf3A
2019/03/25 17:01:41 RelayHttpServer.go:266: relay server address: 0xC27129680f17787061A03CEfEF37E52ef4a0Cf3A
2019/03/25 17:01:41 RelayHttpServer.go:60: RelayHttpServer started.Listening on port: 8090
2019/03/25 17:01:41 relay_server.go:350: abi: cannot marshal in to go type: length insufficient 64 require 96
2019/03/25 17:01:41 RelayHttpServer.go:326: abi: cannot marshal in to go type: length insufficient 64 require 96
2019/03/25 17:01:41 RelayHttpServer.go:329: Waiting for stake...
2019/03/25 17:01:41 relay_server.go:350: abi: cannot marshal in to go type: length insufficient 64 require 96
2019/03/25 17:01:41 RelayHttpServer.go:326: abi: cannot marshal in to go type: length insufficient 64 require 96
2019/03/25 17:01:41 RelayHttpServer.go:329: Waiting for stake...

Relay: manage client/dapp reputation

The relay should maintain a list of rejected calls (that is, relayed calls in which accept_relayed_call returned "true" when called off-chain, but returned an error on-chain, and thus caused the relay to pay for the transactions)
The relay should not completely reject such dApps, since there are edge-cases that might cause such cases.
It should give it some weight, so that a dApp that does it repeatedly will stop getting service.

Implementation depends on #94

Filter on RelayAdded, RelayRemoved separately in RealyClient

Currently, ServerHelper does 'getPastEvents' to get ALL events, and manually filters. Instead, it should execute 'getPastEvents' for RelayAdded' and 'RelayRemoved' events separately and filter out removed relays, since dead relays can no longer be re-added.

relaydock | truffle: command not found

steps to reproduce:

  1. Clone repo, cd tabookey-gasless and yarn
  2. ./dock/run.sh to build the relaydock docker image.
  3. ./dock/run.sh yarn to install dependency.
  4. ./dock/run.sh ./restart-relay.sh

logs below:

echo "Using GOPATH=/home/nervehammer/Desktop/push/tabookey-gasless/server:/home/nervehammer/Desktop/push/tabookey-gasless/server/../build/server"
Using GOPATH=/home/nervehammer/Desktop/push/tabookey-gasless/server:/home/nervehammer/Desktop/push/tabookey-gasless/server/../build/server
mkdir -p /home/nervehammer/Desktop/push/tabookey-gasless/server/../build/server/bin
go build -o /home/nervehammer/Desktop/push/tabookey-gasless/server/../build/server/bin/RelayHttpServer src/RelayHttpServer.go src/utils.go
strip /home/nervehammer/Desktop/push/tabookey-gasless/server/../build/server/bin/RelayHttpServer
make: Leaving directory '/home/nervehammer/Desktop/push/tabookey-gasless/server'
./restart-relay.sh: line 56: truffle: command not found
onexit
./restart-relay.sh: line 15:  1251 Terminated              sh -c "$GANACHE -d |grep ganache-core"

Remove "Failed to retransmit relayed tx: known transaction" error from log

Currently the RelayClient attempts to re-transmit the request it got from the RelayServer (to prevent it from cheating...). As long as the relay is "honest", this sendTransaction will fail - but that's OK.
We should treat this error as "success", and simply not log it (though other errors must still be logged)

Commands "./dock/run.sh yarn" & "/dock/run.sh ./relayserver.sh" are not working.

Hi Guys,

There are some commands that are failing in the latest version of the repository.

  • ./dock/run.sh yarn is failing as yarn is not installed while building a docker image. A workaround could be to use ./dock/run.sh npm install`.

  • ./dock/run.sh ./restart-relay.sh is failing as truffle is not installed while building a docker image.

I am creating pull request to fix these issue.

Relay server sometimes fails to relay transaction.

Relay server sometimes (2/10) throws error No relay responded! 1 attempted, 1 pinged.
Stdout on relay server outputs this -:
relayTransaction: VM Exception while processing transaction: revert GREATER_OR_EQUAL_TO_32_LENGTH_REQUIRED

Suggest that when requesting a signature, we tell users what they are signing

As a suggestion, currently when we are using MetaMask as the tool signing the message, it looks very scary to the user- we are asked to sign something without any context what we are signing.

It would be considerate if when using MetaMask we could tell users what they are signing and in particular tell them that no funds of theirs is at risk.

client excessively read too many logs

When searching for a relay to use for a transaction, the RelayClient scans for all relays that were added/refreshed recently, by searching for RelayAdded events.
The search, however, is done on ALL events from the relay, instead of only RelayAdded (and RelayRemoved) events.
As a consequence, the client actually get (and discards) all TransactionRelayed events.

This has performance issue, and potentially, if there are more than 1000 relayed transaction a day (yay!), it will completely break the client.

What has to be done:

The search is done in ServerHelper.fetchRelaysAdded()

  • it should read only RelayAdded events (instead of allEvents)
  • after processing those events, it should read RelayRemoved, and remove matching relays.

NOTE: a relay can't be added after its removed, so its OK to process all "added" events first, and then all "removed" events.

Make RelayHub a singleton with same address in all networks

Doing this would mean that the hub address can be safely hardcoded in all RelayRecipients, making it easier to implement the interface (as well as simplifying testing, etc.). In the case of a hub upgrade, already existing RelayRecipients will need to be able to point to the new hub anyway, so it is simply a case of changing the new address: it being pre-determined doesn't affect this.

To achieve this, we could use ERC1820's deployment scheme. Basically, the deploy transaction is signed which a synthetic signature, which resolves to an address for which no-one knows the private keys. Deployment is then done by sending funds to this address, and then broadcasting the contract creation tx. Because the account's address is always the same, the smart contract code is the same, and the nonce is 0, the contract address is fixed and known beforehand. A nonce can be added to the source code to obtain a vanity address (e.g. one that starts with 0x1613).

The ERC1820 author included the scripts he used to calculate the nonce for his vanity address in his repo, so this should be super easy to setup: https://github.com/0xjac/ERC1820/tree/master/scripts.

The only thing to keep in mind is that, since the contract bytecode includes a hash of the source code, any changes on the source will also change the deployment address, so it may be the case that the address of the final version of the EIP doesn't match the one in the draft. I think this is not a big issue, however, and a simple warning on the EIP text should suffice.

Invalid json [] when making relayed transaction.

I have been working on adding relayed transactions to a Zepkit tutorial. The relayed transaction works as expected, but throws an error in the logs.

MetaMask - RPC Error: Internal JSON-RPC error. {code: -32603, message: "Internal JSON-RPC error."}
and the relayer throws this in its logs:
2019/03/26 16:04:35 RelayHttpServer.go:185: Invalid json [] unexpected end of JSON input

I followed along the steps here to implement Tabookey's relayed transactions.

It is throwing an error at these lines.
Screen Shot 2019-03-26 at 6 30 06 PM

Conflicting Solc or Truffle versions

I'm getting this error and similar ones. I've been trying different truffle and solc version and I have not found a good way to fix this issue. It also happened in the MetaCoin contract.

Has anyone found a solution to this? Is there something I need to do not mentioned in the README?

Error: Error: CompileError: ParsedContract.sol:1:1: ParserError: Source file requires different compiler version (current compiler is 0.5.8+commit.23d335f2.Emscripten.clang - note that nightly builds are considered to be strictly less than the released version
pragma solidity ^0.4.17;

Add support for relay filtering to the RelayClient

Currently, there is no way to control the selection of a Relay from the client code.
We should start with adding the following configuration options:
"matchUrl": a regex matched against URL of a relay;
"matchOwners": an array of owners used as a filter; only allow relays from those owners;

These parameters allow a client to route all requests through relays he chooses to trust.

Network statistics

Currently the only network status is the ./scripts/gsn-stats
Should add a web-based network status tool.

Code coverage: save data on process exit

Currently, each test file that runs solidity coverage had to handle saving the coverage data by calling past coverage().
But Actually, only the last test for has to do that

So instead, this call skills be scheduled by process.beforeExit() and this is would happen exactly once, when all tests are finished.

Document new network deployment.

Document how to:

  • Deploy new RelayHub
  • Save its address
  • Configure and deploy relay(s) to use the new hub
  • Configure dApp to use the new relay.

Relay Management Tool

Current the management tool (webtools/relaymanager.html) is very basic, and supports a single relay. A more comprehensive web tools is required, to provide more management features:

For a given owner:

  • show all relays.
  • For each relay, check its balance and health (ping) status.
  • Show statistics (some of which are in scripts/gsn-stats
  • Fund (refill relays with ether)
  • Remove relay
  • Launch new lambda/function relays

relay: persistent transaction management

Relay should:

  • Save all sent (pending) transactions.
  • Remove transactions as they are mined.
  • When a relay process is started, check if there are pending transaction (more than X minutes)
  • re-send such pending transaction, with boosted gas price (e.g. eth.gasPrice*1.5)
    Rationale: if a transaction blocks the relay (even if it agreed for a given gas price), it blocks the relay from processing new transactions, so it's better to boost it, even if it's on its own expense.

relayOptions to filter relays

relay selection should have options:
matchUrl : a regex matched against URL of relay
owners: array of owners, used as a filter the owners

fix dangling test

There is a dangling test, that tend to fail circleCI occasionally:
its "should add relay to failedRelay dict in case of http timeout"
Probably it use actual timeout, and fails timing issues on circleCI server makes it fail.

Document relay deployment.

Currently, we have a gsnrelay.zip which explains relay deployment on xdai network.

  • Should document deployment info to the repo (referenced from README)
  • Need to test network first (using gsn-stats.js), to determine RelayHub address
  • should not rely solely on DuckDns: allow deploying relays with static cert, bypassing let's encrypt's 3-month certs.

Can only make 3 transactions before relay fails.

I have a curious error, I have GSN working but I can successfully make only three transactions before I get an error.

The error calls up the debugger in chrome and points me to line 422 of relayClient.js

All of the transactions I make are identical (calling an OpenZeppelin ERC721 contract- mint function). If I try to make the same transaction with Metamask the transaction goes through without a problem.

On the Relay server i get this error:

2019/04/27 15:46:01 RelayHttpServer.go:174: Relay Handler Start
2019/04/27 15:46:01 RelayHttpServer.go:185: Invalid json [] unexpected end of JSON input
2019/04/27 15:46:01 RelayHttpServer.go:77: Checking relay server's ether balance at 0x4989e120D5EDf7C1000FEfc34dC89297f70779d1
2019/04/27 15:46:01 relay_server.go:196: relay server balance: 1081159780000000000
2019/04/27 15:46:01 RelayHttpServer.go:97: Relay received gasPrice:: 200000000
2019/04/27 15:46:01 RelayHttpServer.go:174: Relay Handler Start
2019/04/27 15:46:01 RelayHttpServer.go:189: RelayHubAddress 0x9C57C0F1965D225951FE1B2618C92Eefd687654F
2019/04/27 15:46:01 relay_server.go:480: Checking if canRelay()...
2019/04/27 15:46:01 relay_server.go:813: before CanRelay
2019/04/27 15:46:01 relay_server.go:816: VM Exception while processing transaction: invalid opcode
2019/04/27 15:46:01 relay_server.go:491: can_relay failed in server VM Exception while processing transaction: invalid opcode
2019/04/27 15:46:01 RelayHttpServer.go:192: Failed to relay

2019/04/27 15:46:41 RelayHttpServer.go:276: Waiting for registration...
2019/04/27 15:46:41 relay_server.go:443: relay.RelayHubAddress 0x9C57C0F1965D225951FE1B2618C92Eefd687654F
2019/04/27 15:46:41 RelayHttpServer.go:278: when registered: 1556379740 unix: 2019-04-27 15:42:20 +0000 UTC
2019/04/27 15:46:41 RelayHttpServer.go:287: Trying to get gasPrice from node...
2019/04/27 15:46:41 RelayHttpServer.go:298: GasPrice: 200000000

If I refreh the page and try to make the transaction again with the relay, again it fails, without calling the debugger, but the erorr in the console is this:

relayTransaction: req: from:0x0a342460053e4621f692b14df2e966788350642b
to:0xef6a4c4c3ba9d8d0e6f033433e6fd0f469e15956
encodedFunctionCall:0x50bb4e7f0000000000000000000000000a342460053e4621f692b14df2e966788350642b000000000000000000000000000000000000000000000000000000000000000a0000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000000c666972737420546f6b656e310000000000000000000000000000000000000000
txfee:12
gasPrice:20000000000
gasLimit:5000000
nonce:2
relayhub:0x9C57C0F1965D225951FE1B2618C92Eefd687654F
relayAddress:0x21f7edb587fb400b9a687147fae19d23811d1f70
relayclient.js:393 relayTransaction: [object Object]
App.js:169 Error: No relay responded! 1 attempted, 1 pinged
    at RelayClient._callee3$ (relayclient.js:342)
    at tryCatch (runtime.js:45)
    at Generator.invoke [as _invoke] (runtime.js:264)
    at Generator.prototype.(anonymous function) [as next] (http://localhost:3000/static/js/1.chunk.js:9807:21)
    at asyncGeneratorStep (asyncToGenerator.js:3)
    at _next (asyncToGenerator.js:25)

This is the output in the relay:

2019/04/29 18:23:16 RelayHttpServer.go:152: Sending relayServer eth address
2019/04/29 18:23:16 RelayHttpServer.go:164: address 0x21f7eDB587fB400B9a687147fAE19D23811D1F70 sent
2019/04/29 18:23:16 RelayHttpServer.go:152: Sending relayServer eth address
2019/04/29 18:23:16 RelayHttpServer.go:164: address 0x21f7eDB587fB400B9a687147fAE19D23811D1F70 sent
2019/04/29 18:23:19 RelayHttpServer.go:77: Checking relay server's ether balance at 0x21f7eDB587fB400B9a687147fAE19D23811D1F70
2019/04/29 18:23:19 relay_server.go:196: relay server balance: 1090781700000000000
2019/04/29 18:23:19 RelayHttpServer.go:97: Relay received gasPrice:: 200000000
2019/04/29 18:23:19 RelayHttpServer.go:174: Relay Handler Start
2019/04/29 18:23:19 RelayHttpServer.go:185: Invalid json [] unexpected end of JSON input
2019/04/29 18:23:19 RelayHttpServer.go:77: Checking relay server's ether balance at 0x21f7eDB587fB400B9a687147fAE19D23811D1F70
2019/04/29 18:23:19 relay_server.go:196: relay server balance: 1090781700000000000
2019/04/29 18:23:19 RelayHttpServer.go:97: Relay received gasPrice:: 200000000
2019/04/29 18:23:19 RelayHttpServer.go:174: Relay Handler Start
2019/04/29 18:23:19 RelayHttpServer.go:189: RelayHubAddress 0x9C57C0F1965D225951FE1B2618C92Eefd687654F
2019/04/29 18:23:19 relay_server.go:480: Checking if canRelay()...
2019/04/29 18:23:19 relay_server.go:813: before CanRelay
2019/04/29 18:23:20 relay_server.go:816: VM Exception while processing transaction: invalid opcode
2019/04/29 18:23:20 relay_server.go:491: can_relay failed in server VM Exception while processing transaction: invalid opcode
2019/04/29 18:23:20 RelayHttpServer.go:192: Failed to relay
2019/04/29 18:23:23 RelayHttpServer.go:304: hub to check stake 0x9C57C0F1965D225951FE1B2618C92Eefd687654F
2019/04/29 18:23:23 relay_server.go:430: Stake: 1100000000000000000
2019/04/29 18:23:23 RelayHttpServer.go:316: Checking relay server's ether balance at 0x21f7eDB587fB400B9a687147fAE19D23811D1F70
2019/04/29 18:23:23 relay_server.go:196: relay server balance: 1090781700000000000
2019/04/29 18:23:23 RelayHttpServer.go:327: Relay funded. Balance: 1090781700000000000
2019/04/29 18:23:23 relay_server.go:443: relay.RelayHubAddress 0x9C57C0F1965D225951FE1B2618C92Eefd687654F
2019/04/29 18:23:23 RelayHttpServer.go:334: when registered: 1556561968 unix: 2019-04-29 18:19:28 +0000 UTC
2019/04/29 18:23:23 RelayHttpServer.go:338: Relay registered lately. No need to reregister
2019/04/29 18:23:37 RelayHttpServer.go:304: hub to check stake 0x9C57C0F1965D225951FE1B2618C92Eefd687654F

Not sure what I should be checking here.

Penalize ANY transaction from a relay to ANYTHING BUT a RelayHub

This is done to prevent yet unknown attacks, leaked private keys and overall suspicious behaviour.

  • panalize() contract function should penalize on any transaction that is not relay() or register_relay()
  • The relay itself should verify that it is using new address (with nonce=0), to ensure that there are no previous penalizable transactions.

Use ABIEncoderV2 to work around 'stack too deep' issues

relayCall and recipientCallsAtomic are hard to make changes to because they have too many local variables stored in the stack, causing the compiler to emit 'stack too deep' errors. This is particularly problematic because we actually need to have these functions hold more variables, since some required values are not currently being forwarded, and we still need to rework the charge calculations, which will take some extra slots.

A clean and maintainable way to do achieve this is storing relayCall's arguments in a struct, which are kept in memory. However, since recipientCallsAtomic is an external function, we need to enable the ABIEncoderV2 (an experimental feature) for this to work. This may raise concerns from a security point of view, since this encoder hasn't been as tested as the default one (though our use case is quite straightforward and doesn't require any of its tricky bits).

Additionally, it is likely that usage of the new encoder would bleed over into IRelayRecipient, which means that developers writing contracts that are GSN-callable will get the following warning when compiling their code:

Warning: Experimental features are turned on. Do not use experimental features on live deployments.
pragma experimental ABIEncoderV2;
^-------------------------------^

Are we ok with doing such a thing? I think this is a very reasonable course of action (and the only one I can think of), and fine for an MVP, but wanted to get thoughts and ideas around this.

Contract (recipient) management tool

The minimum functionality a dApp contract developer needs from this tool is the ability to deposit funds for its contract to accept calls.
Current tool (webtools/contractmanager.html) is very basic.

The tool should also allow:

  • View relayed transaction.

  • Show possible rogue clients (clients that deliberately caused relays to pay for failed transaction). These clients should be blacklisted, so they can no longer cause damage.

  • Show current balance (on relay hub)

  • Deposit

  • change (upgrade) RelayHub

  • Auto-deposit - command-line, background tool to automatically fund the contract if needed.

Consider using camelCase instead of snake_case for external methods

Given that the GSN is aimed to be standardized as an EIP, it would be good for its interface to conform to the style guides that apply for both Solidity and Vyper. In particular, all functions should follow camelCase (also known as mixedCase) instead of snake_case. Also, all current standards (such as ERC20 or ERC721) follow this convention.

This would mean changing all function names, such as accept_relayed_call to acceptRelayedCall.

Inconsistent gas_overhead for relayed transaction

Currently, when the RelayHub processes a transaction, the gas_overhead is not fixed: there is a small code that varies with actual relayed transaction size.

EXPECTED RESULT
the gas_overhead should be fixed (indeed, the tests overcome this problem by verifying the rounded precentage, since the actual price is not fixed..

CAUSE
AFTER calculation of the charge, the relay performs keccak(encoded_function)
This has 2 problems:

  • the overhead is not fixed: it depends on the size of the transaction
  • it is completely useless: the hash can't be used for anything, unless the same function is called twice, with the same parameters.
  • This param should be replaced with the encoded function selector, which has some usefulness.

Relays can cause delays to clients and not be penalized

From the EIP:

Attack: Relay attempts to censor a transaction by signing it, but publishing a different
transaction with the same nonce.

Reusing the nonce is the only DoS performed by a Relay, that cannot be detected within a couple of seconds during the http request. It will only be detected when the malicious transaction with the same nonce gets mined and triggers the RelayHub.TransactionRelayed event.

This attack is mitigated by the penalization mechanism, which will let any account penalize the relay, causing it to lose half of its stake. However, this is the actual implementation in the contract:

function penalizeRepeatedNonce(bytes memory unsignedTx1, bytes memory signature1, bytes memory unsignedTx2, bytes memory signature2) public {
    address addr1 = keccak256(abi.encodePacked(unsignedTx1)).recover(signature1);
    address addr2 = keccak256(abi.encodePacked(unsignedTx2)).recover(signature2);

    require(addr1 == addr2, "Different signer");

    Transaction memory decodedTx1 = decodeTransaction(unsignedTx1);
    Transaction memory decodedTx2 = decodeTransaction(unsignedTx2);

    require(decodedTx1.nonce == decodedTx2.nonce, "Different nonce");
    require(keccak256(abi.encodePacked(decodedTx1.data)) != keccak256(abi.encodePacked(decodedTx2.data)), "tx.data is equal");

    penalize(addr1);
}

Two transactions with the same nonce are not enough to cause a penalization: the data field must be different. This achieves two purposes:

  • a transaction cannot be penalized against itself
  • a relay may resend a previously sent transaction using a higher gas price

However, this opens the door to relay attacks. A relay may also:

  • reuse the nonce, with the same data, and a lower gas limit, causing the relayCall call to not succeed due to not enough gas
  • reuse the nonce, with the same data, and give value to it, causing the relayCall call to revert due to the function not being payable

In both scenarios, the relay caused a delay on the client, but it cannot be penalized by the current mechanisms. We need to update the penalizeRepeatedNonce so that these cases are handled.

Relay should never throw

CURRENT BEHAVIOUR:
If can_relay() fails (return non-zero), the relay() method throws, causing the relay to pay for the transaction.
(NOTE: can_relay() is called directly by the relay before submitting the transaction, so the only case where the relay will attempt to create the transaction is if can_relay() succeeded when called off-chain)
In order to let the relays easily find such historical cases (where contract caused any relay to pay for unprocessed transaction) this should be fixed:

SUGGESTED BEHAVIOUR
The relay() function should be "nothrow": in case can_relay() fails, it should emit TransactionFailed( sender,target,relay, can_relay_result ) event, and exit.
Note that even if can_relay reverts, this event should be created.

With this fix, Issue #76 becomes must simpler to implement

Gas limit calculation is broken

When using the RelayProvider, we MUST specify force_gasLimit.
Without it, the client will create a transaction with ZERO gas limit and fail.

Steps to reproduce:

  1. Checkout https://github.com/tabookey/metacoin, branch "with-relay"
  2. remove 'force_gasLimit' value
  3. npm run dev

Expected:
The transaction completes successfully

Actual:
The transaction fails, having 0 as gas limit

NOTE: the default gas should be taken from web3.eth.getGasPrice()

Client: should revert on TransactionFailed

Client's RelayProvider already recognized failed transaction (event TransactionRelayed with status=0) and treat it as failure.
With #94, the relay() transaction doesn't gets reverted in case of contract problem, but it means that the client can behave strangely, as it "thinks" the transaction succeded.
Instead, the RelayProvider should check for TransactionFailed too, and report it as failure.

Subscribing to Events does not work.

On the front end, using the tabookey-gassless provider does not work to subscribe to events.

In testing using this function:

subscribeLogEvent = async (instance, eventName) => {
//Normally I would get my web3 provider from props:
//const {web3} =this.props;

//Instead to make subscriptions work, I need to create a new Web3 provider
    const web3 = new Web3(window.ethereum);
    const eventJsonInterface = web3.utils._.find(
      instance._jsonInterface,
      o => o.name === eventName && o.type === 'event',
    );
    
    const subscription = web3.eth.subscribe('logs', {
      address: instance.options.address,
      topics: [eventJsonInterface.signature]
    }, (error, result) => {
      if (!error) {
        const eventObj = web3.eth.abi.decodeLog(
          eventJsonInterface.inputs,
          result.data,
          result.topics.slice(1)
        )
        console.log(`New ${eventName}!`, eventObj)
        const {message, timestamp, user} = eventObj;
        const msg = {message, timestamp, user};
        this.setState(() => {
          return { ...this.state, messages: [...this.state.messages, msg]  };
        });
      }
    })
    console.log("The Subscription is: ", subscription);
  return subscription;
  }

I need to create a new Web3 provider in order for the subscriptions to work. It would be nice if we could use only one web3 provider for everything rather than create a new provider just for events.

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    ๐Ÿ–– Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. ๐Ÿ“Š๐Ÿ“ˆ๐ŸŽ‰

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google โค๏ธ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.