CIP 55: Remove Tobin Tax

[palango]

Description

Remove tobin tax deduction with next hardfork.

Tested

I tested this manually by running the TestTransferCELO test first without changes. This showed deduction of tobin tax. Then I changed the hardfork flag to always return true and made sure that the tobin tax deduction code is not run anymore.

Related issues

• Fixes https://github.com/celo-org/celo-blockchain-planning/issues/18

Backwards compatibility

Changes consensus rules and will therefor be enabled with the next hardfork.

[akeyless-target-app]

Coverage from tests in ./e2e_test/... for ./consensus/istanbul/... at commit c8da8af

coverage: 49.9% of statements across all listed packages

coverage:  63.0% of statements in consensus/istanbul
coverage:  40.1% of statements in consensus/istanbul/announce
coverage:  54.5% of statements in consensus/istanbul/backend
coverage:   0.0% of statements in consensus/istanbul/backend/backendtest
coverage:  24.3% of statements in consensus/istanbul/backend/internal/replica
coverage:  65.7% of statements in consensus/istanbul/core
coverage:  45.0% of statements in consensus/istanbul/db
coverage:   0.0% of statements in consensus/istanbul/proxy
coverage:  64.4% of statements in consensus/istanbul/uptime
coverage:  51.8% of statements in consensus/istanbul/validator
coverage:  79.2% of statements in consensus/istanbul/validator/random

CommentID: 81142c405e

[github-actions]

Coverage from tests in ./e2e_test/... for ./consensus/istanbul/... at commit 3e61e11

coverage: 48.2% of statements across all listed packages

coverage:  60.0% of statements in consensus/istanbul
coverage:  40.1% of statements in consensus/istanbul/announce
coverage:  54.5% of statements in consensus/istanbul/backend
coverage:   0.0% of statements in consensus/istanbul/backend/backendtest
coverage:  24.3% of statements in consensus/istanbul/backend/internal/replica
coverage:  59.8% of statements in consensus/istanbul/core
coverage:  45.0% of statements in consensus/istanbul/db
coverage:   0.0% of statements in consensus/istanbul/proxy
coverage:  65.0% of statements in consensus/istanbul/uptime
coverage:  51.8% of statements in consensus/istanbul/validator
coverage:  79.2% of statements in consensus/istanbul/validator/random

[github-actions]

[codecov]

Codecov Report

Patch coverage: 66.99% and project coverage change: +1.05 🎉

Comparison is base (71bdbcf) 54.30% compared to head (d1912c1) 55.35%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2029      +/-   ##
==========================================
+ Coverage   54.30%   55.35%   +1.05%     
==========================================
  Files         692      674      -18     
  Lines      115642   113454    -2188     
==========================================
+ Hits        62795    62808      +13     
+ Misses      49014    46835    -2179     
+ Partials     3833     3811      -22     

Impacted Files	Coverage Δ
cmd/devp2p/internal/ethtest/transaction.go	0.00% <0.00%> (ø)
cmd/geth/chaincmd.go	0.00% <0.00%> (ø)
cmd/geth/main.go	21.39% <ø> (+0.91%)	⬆️
cmd/geth/usage.go	10.90% <ø> (ø)
cmd/utils/flags.go	2.56% <0.00%> (ø)
consensus/istanbul/utils.go	47.10% <ø> (+1.13%)	⬆️
contracts/currency/currency.go	55.00% <ø> (ø)
contracts/election/election.go	25.00% <ø> (ø)
contracts/epoch_rewards/epoch_rewards.go	88.88% <ø> (ø)
contracts/freezer/freezer.go	70.00% <ø> (ø)
... and 48 more

... and 27 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[eelanagaraj]

LG! Can you add a second test case, i.e. (TestTransferCELOPreG, TestTransferCELOPostG) which transfer before/after the G hardfork and checks expected behavior? I think it'd be really useful to codify your testing (from the description) & have this going forwards to make sure we don't inadvertently mess with the old logic in the future. Wdyt? Happy to approve after that!

As far as your question/comment, I think maybe slightly preferable to move the debug setting to the top of the func (for easier modification later when redoing all the SharedEVMRunner trace fixes) but I think totally up to you!

[palango]

I think it'd be really useful to codify your testing (from the description) & have this going forwards to make sure we don't inadvertently mess with the old logic in the future. Wdyt? Happy to approve after that!

I agree it would be super useful, but the only way I see to do it involves a full e2e test at the moment, because we don't have the necessary setup logic for units/integration tests to setup the whole celo contract infrastructure.

And even the e2e setup is quite involved as it requires the reserve to be setup in a way to trigger the tobin tax payment, besides the whole governance setup.

[karlb]

Looks straightforward enough.