corpus-io / tokenize.it-smart-contracts

These are the smart contracts used in tokenize.it, along with documentation.
GNU Affero General Public License v3.0
24 stars 4 forks source link

introduce meta tx #1

Closed malteish closed 2 years ago

malteish commented 2 years ago

Background

For (new) users to be able to use ContinuousFundraising and PersonalInvite, we leverage ERC2771 through gas station network. This requires changes to both contracts.

For transfers, updates on Token are also necessary.

malteish commented 2 years ago

Using EIP2771Context instead of BaseRelayRecipient because it works with the Openzeppelin contracts we inherit from: https://docs.opengsn.org/faq/troubleshooting.html#my-contract-is-using-openzeppelin-how-do-i-add-gsn-support

malteish commented 2 years ago

Research:

malteish commented 2 years ago

@CJentzsch hints in #5: The buy and deal function in PersonalInvite.sol and ContinuousFundraising.sol should be callable through metatransactions.

See https://opengsn.org/, https://docs.openzeppelin.com/contracts/4.x/api/metatx#MinimalForwarder, https://eips.ethereum.org/EIPS/eip-2612 and https://docs.openzeppelin.com/contracts/4.x/api/token/erc20#ERC20Permit for more details.

malteish commented 2 years ago

Flow of meta transaction calls to investment contracts (e.g. ContinuousFundraising or PersonalInvite):

can the permit be included in the same meta-tx the user signs for step 2 onwards? Seems unlikely, it might need to be a separate step (like with allow).

malteish commented 2 years ago

Background reading:

malteish commented 2 years ago

Christoph says:

Habe mir mal die GSN contracts genau angeschaut. Sie sind wirklich gut. Leider ist die aktuelle Fassung nicht geauditet :face_with_rolling_eyes: Folgende Überlegungen: Um unsere contracts metatx-fähig zu machen können wir entweder vom OpenZeppelin contract: https://github.com/OpenZeppelin/openzeppelin-contracts/blob/v4.7.3/contracts/metatx/ERC2771Context.sol (was du in deinen PR draft gemacht hast) erben oder vom GSN contract: https://github.com/opengsn/gsn/blob/master/packages/contracts/src/ERC2771Recipient.sol Beide machen dasselbe, außer des GSN statt im Construtor den TrustedForwarder zu nutzen, eine explizite Funktion _setTrustedForwarder zur Verfügung stellt die man dann direkt im Constructor von unseren Contract aufrüfen müßte, und die es prinzipiell ermöglicht (wenn wir das in unseren Contract implementieren) den TrustedForwarder auszutauschen (gefährlich …) ERC2771Context.sol https://github.com/[OpenZeppelin/openzeppelin-contracts](https://github.com/OpenZeppelin/openzeppelin-contracts)|OpenZeppelin/openzeppelin-contractsOpenZeppelin/openzeppelin-contracts | Added by GitHub ERC2771Recipient.sol https://github.com/[opengsn/gsn](https://github.com/opengsn/gsn)|opengsn/gsnopengsn/gsn | Added by GitHub Außerdem habe ich mit den MinimalForwarder von OpenZeppelin und den von GSN angeschaut. Der vom GSN hat ein paar Security features dich wirklich gut sind, zum Beispiel: replay protection on other chains (called Domains in the contract) send back unused ETH (in case more ETH was send with the transaction then forwarded as specified in the value parameter, that ETH is send back to the sender. Das ist alles sehr nützlich. Von daher würde ich den vom GSN nutzen.

christoph 10:14 AM Wir sollten das konsequent in allen Verträgen einsetzen. Also auch im Token contract (hier sollten die settings via MetaTx zu setzen sein, und wir brauchen hier auch permit .

malteish commented 2 years ago

ERC2771

Implementation of ERC2771 to extend our contracts with

Forwarder

Paymaster

A custom Paymaster contract is necessary. It could decide to accept and pay for transactions from Allowlisted users that want to interact with the tokenize.it-contracts. Concerning security this is not very critical: The worst that can happen in an atttack is that the Paymaster pays for transactions we didn't want to pay for, until it is drained. Then meta-tx don't work anymore until the paymaster is fixed through updates and refunded (meaning it receives new funds to once again pay for user's gas fees). This does not affect:

This would work as 1 Paymaster for 1 Allowlist for many tokens. Nice. It doesn't cover tx fees for addresses that have the transferer role in the token contracts, but maybe that is okay. Maybe transferers can pay tx fees anyway.

ERC2612

The currencies that should be usable still need examination, see #10. If they all implement ERC2612, but not ERC2771, we should consider making our token compatible with that approach.

CJentzsch commented 2 years ago

Regarding ERC2771Recipient.sol vs ERC2771Context.sol: