Implemented the limit per fee currency

carterqw2 commented 11 months ago

Description

In the light of the recent growth of interest in extending of the gas currencies allowlist, we need to implement a protective mechanism that would allow validators more precisely control the percentage of available block space used paid with an alternative fee currency (other than CELO).

Goals

Allow validators to limit the block space allocated for transactions paid with alternative fee currencies (with a per currency granularity if needed).
Minimize the effort required for this feature:
- We want people to use this feature more, not less, so the limiting should be permissive by default.
- The allowlist is dynamic, it shouldn’t be required to change any configuration parameters when a new currency gets added to the list.

Proposed Solution

Add a flag celo.feecurrency.limits with a comma-separated currency address hash=limit mappings for currencies listed in the FeeCurrencyWhitelist contract, where limit represents the maximal fraction of the block gas limit as a float point number available for the given fee currency.

For example, --celo.feecurrency.limits=0x765DE816845861e75A25fCA122bb6898B8B1282a=0.1,0xD8763CBa276a3738E6DE85b4b3bF5FDed6D6cA73=0.05;0xEd6961928066D3238134933ee9cDD510Ff157a6e=0.

Add an overridable default value celo.feecurrency.default (initially set to 0.5 ) for currencies not listed in the map, meaning that if not specified otherwise, a TX with a given fee currency can take up to 50% of the block space.

CELO token doesn’t have a limit.

Default Configuration

Based on historical data we will use this default configuration:

--celo.feecurrency.default=0.5
--celo.feecurrency.limits=0x765DE816845861e75A25fCA122bb6898B8B1282a=0.9,0xD8763CBa276a3738E6DE85b4b3bF5FDed6D6cA73=0.5,0xe8537a3d056DA446677B9E9d6c5dB704EaAb4787=0.5

It imposes the following limits:

cUSD up to 90% cEUR up to 50% cREAL up to 50% any other token except CELO - 50%

Other changes

No.

Tested

Added tests.

Backwards compatibility

Yes, both flags have default options. Behavior-wise, the effect should be similar to what we observe now.

It would be easier to review by following the commits.

github-actions[bot] commented 11 months ago

5877 passed, 4 failed, 45 skipped

Test failures:
TestTransactionIndices: core `blockchain_test.go:1946: Oldest indexded block mismatch, want 69, have 100`
TestEthersJSCompatibility: e2e_test Checking getExchangeSpenders. spenders = [0x000000000000000000000000000000000000d028] Checking medianRate. numerator = 1000000000000000000000000 denominator = 1000000000000000000000000 Checking gas price minimum. cusdValue = 100000000 executing mocha test with /usr/bin/npm run --networkaddr=http://127.0.0.1:39703 test --blocknum=0x0 -- --grep ethers.js compatibility tests with state > ethersjs-api-check@1.0.0 test /runner/_work/celo-blockchain/celo-blockchain/e2e_test/ethersjs-api-check > mocha -r ts-node/register test/.ts "--grep" "ethers.js compatibility tests with state" ethers.js compatibility tests with state ✔ provider.getBlock works (block has gasLimit set) ✔ EIP-1559 transactions supported (can get feeData) ✔ block has gasLimit 1) block has baseFeePerGas 3 passing (110ms) 1 failing 1) ethers.js compatibility tests with state block has baseFeePerGas: AssertionError: expected false to be true + expected - actual -false +true at /runner/_work/celo-blockchain/celo-blockchain/e2e_test/ethersjs-api-check/test/test.ts:52:10 at Generator.next () at fulfilled (test/test.ts:5:58) at processTicksAndRejections (internal/process/task_queues.js:95:5) npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! ethersjs-api-check@1.0.0 test: `mocha -r ts-node/register test/.ts "--grep" "ethers.js compatibility tests with state"` npm ERR! Exit status 1 npm ERR! npm ERR! Failed at the ethersjs-api-check@1.0.0 test script. npm ERR! This is probably not a problem with npm. There is likely additional logging output above. npm ERR! A complete log of this run can be found in: npm ERR! /home/runner/.npm/_logs/2023-12-07T15_07_08_800Z-debug.log e2e_test.go:685:
TestEthersJSCompatibilityDisableAfterGingerbread: e2e_test `Checking getExchangeSpenders. spenders = [0x000000000000000000000000000000000000d028] Checking medianRate. numerator = 1000000000000000000000000 denominator = 1000000000000000000000000 Checking gas price minimum. cusdValue = 100000000 e2e_test.go:722: e2e_test.go:722: e2e_test.go:722: e2e_test.go:722: e2e_test.go:722: e2e_test.go:724:`
TestEthersJSCompatibilityDisableBeforeGingerbread: e2e_test `Checking getExchangeSpenders. spenders = [0x000000000000000000000000000000000000d028] Checking medianRate. numerator = 1000000000000000000000000 denominator = 1000000000000000000000000 Checking gas price minimum. cusdValue = 100000000 e2e_test.go:770: Checking getExchangeSpenders. spenders = [0x000000000000000000000000000000000000d028] Checking medianRate. numerator = 1000000000000000000000000 denominator = 1000000000000000000000000 Checking gas price minimum. cusdValue = 100000000`
_{This test report was produced by the test-summary action. Made with ❤️ in Cambridge.}

codecov[bot] commented 11 months ago

Codecov Report

Attention: 48 lines in your changes are missing coverage. Please review.

Comparison is base (6731538) 55.09% compared to head (347d8bc) 55.09%.

Files	Patch %	Lines
cmd/utils/flags.go	0.00%	25 Missing :warning:
miner/block.go	44.82%	14 Missing and 2 partials :warning:
core/sys_context.go	0.00%	7 Missing :warning:

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #2203 +/- ## ======================================= Coverage 55.09% 55.09% ======================================= Files 682 683 +1 Lines 114449 114533 +84 ======================================= + Hits 63055 63106 +51 - Misses 47501 47536 +35 + Partials 3893 3891 -2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

github-actions[bot] commented 11 months ago

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

coverage: 46.9% of statements across all listed packages

coverage:  57.2% of statements in consensus/istanbul
coverage:  23.7% of statements in consensus/istanbul/announce
coverage:  54.3% 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.1% 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.2% 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

celo-org / celo-blockchain