Implement token interactions #16
Conversation
HristiyanG
commented
Nov 30, 2020
•
HristiyanG
commented
- rework [Cashier] requestCreateOrder (renamed to requestCreateOrder_ETH_ETH)
- accepts array instead of 6 params
- refactored flow as in some cases will accept too many params and will hit "stack too deep"
- [Cashier] requestVoucher renamed to requestVoucher_ETH_ETH
- removed event LogVoucherDelivered from [Cashier] into [VoucherKernel] to keep it consistent as all other voucher logs are emitted from the VoucherKernel
- created functions
- requestCreateOrder_TKN_TKN_WithPermit
- requestCreateOrder_ETH_TKN_WithPermit
- requestCreateOrder_TKN_ETH_WithPermit
- requestVoucher_TKN_TKN_WithPermit
- requestVoucher_ETH_TKN_WithPermit
- requestVoucher_TKN_ETH_WithPermit
- withdraw is refactored to pay to participants based on tokensupply payment method
- added tests and refactored old ones to call the proper functions, or to look for LogVoucherDelivered from the [VoucherKernel] already
contracts/BosonToken.sol
Outdated
| contract BosonToken is ERC20WithPermit, AccessControl { | ||
| // SPDX-License-Identifier: MIT | ||
|
|
||
| address private owner; |
There was a problem hiding this comment.
Can we use Ownable here from OpenZeppelin? If not we would need to also implement transferOwnership - and the other functions there
There was a problem hiding this comment.
Or we can use the Admin from the AccessControl instead of owner.
There was a problem hiding this comment.
this is fixed in #15
contracts/ERC20WithPermit.sol
Outdated
| mapping(address => mapping(address => uint256)) public override allowance; | ||
|
|
||
| bytes32 public override DOMAIN_SEPARATOR; | ||
| bytes32 public override constant PERMIT_TYPEHASH = 0x6e71edae12b1b97f4d1f60370fef10105fa2faae0126114a169c64845d6126c9; |
There was a problem hiding this comment.
Sorry probably we had some misunderstanding on the previous PR. I think it would be helpful if we have the comment here, how this permit hash is constructed, otherwise it just looks like magic.
contracts/VoucherKernel.sol
Outdated
|
|
||
| address public cashierAddress; //address of the Cashier contract | ||
|
|
||
| mapping(bytes32 => Promise) public promises; //promises to deliver goods or services | ||
| mapping(address => uint256) public tokenNonces; |
There was a problem hiding this comment.
Can you add a comment here why the Nonces are used
contracts/Cashier.sol
Outdated
| emit LogOrderCreated(tokenIdSupply, msg.sender, metadata[5]); | ||
| } | ||
|
|
||
| function requestCreateOrder_TKN_ETH_WithPermit( |
There was a problem hiding this comment.
Here, we do not have a Permit, probably wrong name of the function
| emit LogOrderCreated(tokenIdSupply, msg.sender, metadata[5]); | ||
| } | ||
|
|
||
| function requestCreateOrder_TKN_TKN_WithPermit( |
There was a problem hiding this comment.
I think we need one more set of TKN functions, for tokens that do not support permit - you can use USDC/USDT/DAI as an example, as probably they would be used here.
There was a problem hiding this comment.
This feature has a separate task. It's not planned for the ongoing sprint.
| emit LogVoucherDelivered(_tokenIdSupply, voucherTokenId, _issuer, msg.sender, voucherKernel.getPromiseIdFromVoucherId(voucherTokenId)); | ||
| } | ||
|
|
||
| function requestVoucher_TKN_TKN_WithPermit( |
There was a problem hiding this comment.
Same as above, set of functions to support different cases for ETH_TKN, TKN_TKN, TKN_ETH without Permit
There was a problem hiding this comment.
This feature has a separate task. It's not planned for the ongoing sprint.
