search

hop-protocol / contracts

🐰 Hop Protocol v1 smart contracts
https://hop.exchange
238 stars 98 forks source link

Update L2_Bridge.commitTransfer delay in README.md #93

Closed sz-piotr closed 2 years ago

sz-piotr commented 2 years ago

The README says 100 txs or 4 hours but the code requires 1 day and doesn't count transactions. https://github.com/hop-protocol/contracts/blob/f4c4746ab2be3eb7107d8f031ca3c501a69bd79f/contracts/bridges/L2_Bridge.sol#L181 https://github.com/hop-protocol/contracts/blob/f4c4746ab2be3eb7107d8f031ca3c501a69bd79f/contracts/bridges/L2_Bridge.sol#L27

shanefontaine commented 2 years ago

Thank you for the contribution!

You will find that the function is called after x transactions here. The number is currently 512 txs, not 100, so the README should be updated accordingly to reflect that!

As for the timing, this is a discrepancy between the master branch and the v1 branch.

If you want to update the PR to include the 512 and the 1 day language, then I'll be happy to merge it!

sz-piotr commented 2 years ago

Hey @shanefontaine. You're right there is an option to call _commitTransfer (NOT commitTransfer) via send after 128 (not 512) transactions. However calling sends results in sending yet another transfer. Maybe the wording could be:

- 128 transactions are sent or `L2_Bridge.commitTransfer()` is called by anyone after 1 day
shanefontaine commented 2 years ago

I think that is great and am very happy with that update and 512.

I know the contracts say 128 transactions, but on-chain they have been updated to 512. I have updated the master branch to reflect this with commit bfa90f4cfb67935821f8e11453f84578d8bd0ae8.

Thank you for bringing this to our attention.

sz-piotr commented 2 years ago

Thanks @shanefontaine. I have added another commit that updates this value in the README.

shanefontaine commented 2 years ago

LGTM. Thank you for your contribution!