[hardfork] collect txns fees to a specific account

[peekpi]

Issue

#4261

After FeeCollectEpoch, all the transactions fees will be collected into the feeCollector account instead of be burned. If the feeCollector account is a contract, it will not trigger any of its functions(fallback()/receive() in solidity). feeCollector’s balance increases after each transaction is executed.

[rlan35]

looks good. How is this change tested? We need to at least test on localnet and testnet before launching on mainnet since it's token-related critical change.

[sophoah]

@here, my ask here is to have a new test written in https://github.com/harmony-one/harmony-test so this feature can be tested at every new PR and be part of our CI/CD thanks.

[peekpi]

looks good. How is this change tested? We need to at least test on localnet and testnet before launching on mainnet since it's token-related critical change.

because testnet has not been fixed, we can only test it in localnet. will post test log later.

[peekpi]

test js script:

// fee.js
const ethers = require('ethers')

const provider = new ethers.providers.JsonRpcProvider('http://127.0.0.1:9500')
const feeCollector = '0x19E7E376E7C213B7E7e7e46cc70A5dD086DAff2A'
const faucetPrivateKey = ''
const faucetWallet = new ethers.Wallet(faucetPrivateKey, provider)

async function main() {
    const getBalance = async ()=>({
        feeCollector: await provider.getBalance(feeCollector).then(ethers.utils.formatEther),
        faucet: await faucetWallet.getBalance().then(ethers.utils.formatEther)
    })

    const beforeBalance = await getBalance()
    const gasPrice = ethers.utils.parseEther('0.001')  // 21000 * 0.001 ether = 21 ether
    await faucetWallet.sendTransaction({to:faucetWallet.address, gasPrice}).then(tx=>tx.wait())
    const afterBalance = await getBalance()
    console.log('beforeBalance:', beforeBalance)
    console.log('afterBalance :', afterBalance)
}

main()

run node fee.js after FeeCollectEpoch and it will send a transaction with gas fee of 21 ether. we can see the balance of feeCollector increases.

log:

beforeBalance: { feeCollector: '0.0', faucet: '10000000000.0' }
afterBalance : { feeCollector: '21.0'', faucet: '9999999979.0' }

[MaxMustermann2]

Can you move the checks here to be inside mustValid ? https://github.com/harmony-one/harmony/blob/7c1c43ecbd00db4481ae0c19871c170761b6aa0c/internal/params/config.go#L706-L715

Secondly, can you ensure the FeeCollectEpoch variable is set in all ChainConfig objects in the file ? For example, it is currently missing in StressnetChainConfig.

[peekpi]

Can you move the checks here to be inside mustValid ?

https://github.com/harmony-one/harmony/blob/7c1c43ecbd00db4481ae0c19871c170761b6aa0c/internal/params/config.go#L706-L715

Secondly, can you ensure the FeeCollectEpoch variable is set in all ChainConfig objects in the file ? For example, it is currently missing in StressnetChainConfig.

fixed

[sophoah]

@peekpi will that work for shard 1/2/3 fees ? cc @MaxMustermann2

Also let's assume one day we need a 5th shard, will that still work ?

here are some test case scenario : fee on a tx between s0 to s0 fees on a tx between s1 to s1 fees on a tx between s0 to s1

What about future cross shard contract transaction fee ?

[MaxMustermann2]

What about future cross shard contract transaction fee ?

I can confirm that this will work.

[peekpi]

@peekpi will that work for shard 1/2/3 fees ? cc @MaxMustermann2

Also let's assume one day we need a 5th shard, will that still work ?

here are some test case scenario : fee on a tx between s0 to s0 fees on a tx between s1 to s1 fees on a tx between s0 to s1

What about future cross shard contract transaction fee ?

fee are always collected in src shard. s0->s0: s0 s1->s1: s1 s0->s1: s0 s1->s0: s1

for future cross shard contract transaction, we need to handle this scenario carefully. fee collection takes place in function collectGas(). cc @MaxMustermann2

[MaxMustermann2]

If the FeeCollector is going to be a multisig, #4165 must be activated at the same time (or before) this change. This will allow movement of funds across shards through the smart contract, something that is not currently possible.

[peekpi]

If the FeeCollector is going to be a multisig, #4165 must be activated at the same time (or before) this change. This will allow movement of funds across shards through the smart contract, something that is not currently possible.

does pr #4165 only support cross shard transfer? or also support cross shard contract call?

[MaxMustermann2]

does pr #4165 only support cross shard transfer? or also support cross shard contract call?

It supports only transfers. And it's actually disabled for smart contract (although that needs some more hardening in my opinion). Please ignore my original comment then.