search

AbdelStark / token-vesting-contracts

ERC20 token vesting smart contracts 💰⏳
Apache License 2.0
193 stars 135 forks source link

Update to Solidity 0.8.11, remove mocking #3

Closed vpavlin closed 2 years ago

vpavlin commented 2 years ago

Hi @abdelhamidbakhta! I came across your vesting contracts when I was doing research for a client. I noticed you are using a mock contract to modify the time.

I believe modifying time directly on the chain is a better approach (unless you want to mess with the time on testnet, probably).

I also updated the contracts to the latest Solidity.

Let me know what you think

AbdelStark commented 2 years ago

Hi @abdelhamidbakhta! I came across your vesting contracts when I was doing research for a client. I noticed you are using a mock contract to modify the time.

I believe modifying time directly on the chain is a better approach (unless you want to mess with the time on testnet, probably).

I also updated the contracts to the latest Solidity.

Let me know what you think

Hey. Thanks for the contribution. Yeah basically I did this mocking system to simulate corner cases on testnet. Also I wanted to let the possibility to extend the contract to have a custom function that provides time. The time is somewhat manipulable on some EVM chains. Maybe some companies want to use oracle based timing function depending on legal contract they have with investors for example. So I prefer to keep this option for the moment. Could you submit a PR with the solidity version bump only ?

vpavlin commented 2 years ago

That makes sense, thanks for the explanation.

New PR: https://github.com/abdelhamidbakhta/token-vesting-contracts/pull/4

AbdelStark commented 2 years ago

That makes sense, thanks for the explanation.

New PR: #4

Thank you very much. I merged the new PR