Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[core] fix: Use time-based ordering to avoid spam #4118

Merged
merged 1 commit into from
Apr 4, 2022

Conversation

MaxMustermann2
Copy link
Contributor

Closes #4113 by sorting transactions by time received
instead of using a hashmap based transaction ordering. This sorting is
on top of the gas price and nonce sorting already implemented.

Effectively, this allows the production of almost a deterministic order
of transaction ordering, as opposed to a heap-based hash map ordering
which is affected by Golang's internal operations. Consequently,
arbitrageurs, who spam the network with a view to exist around
arbitrag-able transactions, will have to focus on latency instead of
network spamming.

See also bnb-chain/bsc#269 and ethereum/go-ethereum#21358 for related
issues in other chains.

Issue

#4113

Test

Unit Test Coverage

Before:

core/test: 19.4%

After:

core/test: 19.6%

Test/Run Logs

Ran the following contract + brownie script to observe that time based ordering is dependent on latency and that all transactions go through as expected.

//SPDX-License-Identifier: Unlicense
pragma solidity ^0.8.0;

contract SampleContract {
    address deployer;
    address setter;
    event SampleEvent(bool result);

    constructor() {
        deployer = msg.sender;
        setter = msg.sender;
    }

    function sampleFunction() public {
        bool result = false;
        if (setter == deployer) {
            setter = msg.sender;
            result = true;
        }
        emit SampleEvent(result);
    }
}
from brownie import (
    accounts,
    SampleContract,
    web3
)
from pyhmy import (
    signing,
    transaction,
    util
)
import multiprocessing
import pytest
from eth_utils import to_checksum_address
import time

@pytest.fixture( scope = "module", autouse = True )
def setup():
    accounts.add( '1f84c95ac16e6a50f08d44c7bde7aff8742212fda6e4321fde48bf83bef266dc' )
    print( "Deploying contract" )
    SampleContract.deploy( { 'from': accounts[ 0 ] } )
    print( "Contract ready" )

def send_tx( signed_tx ):
    hash = transaction.send_raw_transaction( signed_tx.rawTransaction.hex() )
    return hash

def check_result( tx_hash ):
    receipt = web3.eth.wait_for_transaction_receipt( tx_hash )
    print( receipt[ 'logs' ][ -1 ][ 'data' ][ -1 ] )
    if receipt[ 'logs' ][ -1 ][ 'data' ][ -1 ] == '1':
        from_address = util.convert_one_to_hex( transaction.get_transaction_by_hash( tx_hash )[ 'from' ] )
        for i, account in enumerate( accounts ):
            print( account.address, from_address )
            if to_checksum_address( account.address ) == to_checksum_address( from_address ):
                print( 'Transaction from account {} went through'.format( i ) )

def test_sample_function():
    signed_txs = []
    # get the latest instance
    sample_contract = SampleContract[ -1 ]
    sample_contract_w3 = web3.eth.contract(
        abi = SampleContract.abi,
        address = to_checksum_address( sample_contract.address )
    )
    pks = [
        # needs PKs
    ]
    # add 10 fake accounts
    for i in range( 1, 11 ):
        print( "Running for account", i )
        if len( accounts ) <= i:
            accounts.add( pks[ i - 1 ][ 2: ] )
        account = accounts[ i ]
        accounts[ 0 ].transfer( account, amount = web3.toWei( 5, 'ether' ) )
        tx_dict = sample_contract_w3.functions.sampleFunction(
            ).buildTransaction(
                {
                    'nonce': 0,
                    'gasPrice': 31 * int( 1e9 ) * 1000, # 31 gwei
                    'gas': 100000, # 100k
                    'chainId': 2,
                    'shardID': 0,
                    'toShardID': 0,
                    'value': 0,
                }
            )
        signed_tx = signing.sign_transaction( tx_dict, account.private_key[ 2: ] )
        signed_txs.append( signed_tx )
    print( "Running pooled transactions" )
    with multiprocessing.Pool( len( signed_txs ) ) as p:
        results = p.map( send_tx, signed_txs )
    print( "Done" )
    for tx_hash in results:
        check_result( tx_hash )
    # Extra buffer on top
    time.sleep( 5 )

Operational Checklist

  1. Does this PR introduce backward-incompatible changes to the on-disk data structure and/or the over-the-wire protocol?. (If no, skip to question 8.)
    No.

  2. Describe the migration plan.. For each flag epoch, describe what changes take place at the flag epoch, the anticipated interactions between upgraded/non-upgraded nodes, and any special operational considerations for the migration.

  3. Describe how the plan was tested.

  4. How much minimum baking period after the last flag epoch should we allow on Pangaea before promotion onto mainnet?

  5. What are the planned flag epoch numbers and their ETAs on Pangaea?

  6. What are the planned flag epoch numbers and their ETAs on mainnet?

    Note that this must be enough to cover baking period on Pangaea.

  7. What should node operators know about this planned change?

  8. Does this PR introduce backward-incompatible changes NOT related to on-disk data structure and/or over-the-wire protocol? (If no, continue to question 11.)
    No.

  9. Does the existing node.sh continue to work with this change?

  10. What should node operators know about this change?

  11. Does this PR introduce significant changes to the operational requirements of the node software, such as >20% increase in CPU, memory, and/or disk usage?
    No.

Closes harmony-one#4113 by sorting transactions by time received
instead of using a hashmap based transaction ordering. This sorting is
on top of the gas price and nonce sorting already implemented.

Effectively, this allows the production of almost a deterministic order
of transaction ordering, as opposed to a heap-based hash map ordering
which is affected by Golang's internal operations. Consequently,
arbitrageurs, who spam the network with a view to exist around
arbitrag-able transactions, will have to focus on latency instead of
network spamming.

See also bnb-chain/bsc#269 and ethereum/go-ethereum#21358 for related
issues in other chains.
@MaxMustermann2
Copy link
Contributor Author

Adding logs for the test run below:

Go run, to demonstrate that transactions do occur in the order in which they are received. Note how close the time is between these transactions on a fairly below average machine. Obviously, these times are for the block leader only and other participants may receive the transactions at a different time. Since transaction ordering is not validated, this is a non issue.

Processing transaction 0xce4ef6ca750c1af7856b15db2db4e279df0f61b5a89de7ebdf185f93f16dc90b received at 2022-04-05 00:58:14.647455681 +0200 CEST m=+160.131824870
Processing transaction 0xa8ad6a9d08be3c4a2d8bd18655860be64e3246e736305ceb926e93f5e37e5b00 received at 2022-04-05 00:58:14.652728317 +0200 CEST m=+160.137097501
Processing transaction 0x401cd92b2fd648f96feb0b07f3ddb8aa2ea4f655c79ec43d87517e10883984e3 received at 2022-04-05 00:58:14.655642948 +0200 CEST m=+160.140012117
Processing transaction 0xe40247977f51249b7b364175e2a17884d304ecae856d4dd9cdb9f6205a799562 received at 2022-04-05 00:58:14.65864659 +0200 CEST m=+160.143015762
Processing transaction 0x4f2a72871d69dc6ab656b1804b05fb9271e5fbbe1a58a99a27ba3d24b31a1ab6 received at 2022-04-05 00:58:14.663412122 +0200 CEST m=+160.147781304
Processing transaction 0xd1bc4ee4a9d3c006836e6848146b64658b326717c3d83a0f007bddd0b841ef3f received at 2022-04-05 00:58:14.666243515 +0200 CEST m=+160.150612696
Processing transaction 0x8ab13ed0029b86909c7048809c4a2916d3e3c23c50e68151d85e67df1ab2387c received at 2022-04-05 00:58:14.666458291 +0200 CEST m=+160.150827477
Processing transaction 0xeb94a856ba09e121c4ee42db8393a26415ac5d25f3f2d40288b8b46064f62d91 received at 2022-04-05 00:58:14.668264247 +0200 CEST m=+160.152633440
Processing transaction 0xabaa77f95413c168a16a2646eb084df9065f85da05c2a0978621c1584873571e received at 2022-04-05 00:58:14.668621685 +0200 CEST m=+160.152990866
Processing transaction 0xa71db59e3418a28235dfb5f07274cf5790ac12384453c58fd64593bbf28817b0 received at 2022-04-05 00:58:14.685508338 +0200 CEST m=+160.169877531

Python, to confirm that the transaction which went through first according to Go, actually did. I have used a Solidity variable + event to demonstrate this.

Transaction from account 2 went through with hash 0xce4ef6ca750c1af7856b15db2db4e279df0f61b5a89de7ebdf185f93f16dc90b

@rlan35 rlan35 merged commit 73b4bdf into harmony-one:main Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prevent transaction spamming by fixing hashmap-based transaction ordering
2 participants