pending transactions not applied until after a ChainHeadEvent

[cdetrio]

System information

Version: 1.5.0-unstable-4f4e1026

Expected behaviour

A pending transaction should be applied immediately, and included in the first block mined after the transaction is received.

Actual behaviour

Pending transactions are not applied immediately. After a tx is added to the pending pool, an empty block must be mined first. After an empty block is mined, pending transactions are then included in the second block.

log from worker.go with debug prints:

I0808 20:59:28.111320 internal/ethapi/api.go:1127] Tx(0x9e1155b6553fd108349cec30b0bd004772638718ef1713549245659076b86abe) to: 0x38683b0002f98ad0d8d0f00cc77ebdc48fc0de9e
### ^^ tx recieved!
worker.go case core.TxPreEvent. self.mining:
(int32) 1
I0808 20:59:55.967272 miner/worker.go:348] 🔨  Mined block (#9693 / c8915e7d). Wait 5 blocks for confirmation
### ^^ empty block mined! 27 seconds after tx was received..
worker.go wait(). just mined a block, calling commitNewWork.
worker.go case core.ChainHeadEvent. invoking commitNewWork
### ^^ tx only applied after a new block was mined
worker.go commitNewWork() called. invoking work.commitTransactions() with transactions:
(types.Transactions) (len=1 cap=1) {
 (*types.Transaction)(0xc839f7ab40)(
    TX(9e1155b6553fd108349cec30b0bd004772638718ef1713549245659076b86abe)
    Contract: false
    From:     e9a4690662e26219d11315fe8389b40263a9abc3
    To:       38683b0002f98ad0d8d0f00cc77ebdc48fc0de9e
    Nonce:    104
    GasPrice: 20000000000
    GasLimit  90000
    Value:    0
    Data:     0xf8a8fd6d
    V:        0x1c
    R:        0xcbf2fb79f27f5d0fcf1e404cc4f0ba5bcab7106a198416c7cdde0d336120aa04
    S:        0x1a6913b86ecb5b36bd233e386e93463049ddb9efd88b0eebbe0b20f0ab39efa1
    Hex:      f869688504a817c80083015f909438683b0002f98ad0d8d0f00cc77ebdc48fc0de9e8084f8a8fd6d1ca0cbf2fb79f27f5d0fcf1e404cc4f0ba5bcab7106a198416c7cdde0d336120aa04a01a6913b86ecb5b36bd233e386e93463049ddb9efd88b0eebbe0b20f0ab39efa1
)
}
I0808 20:59:55.968991 miner/worker.go:586] commit new work on block 9694 with 1 txs & 0 uncles. Took 1.66634ms
worker.go commitNewWork() called. invoking work.commitTransactions() with transactions:
(types.Transactions) (len=1 cap=1) {
 (*types.Transaction)(0xc839f7ab40)(
    TX(9e1155b6553fd108349cec30b0bd004772638718ef1713549245659076b86abe)
    Contract: false
    From:     e9a4690662e26219d11315fe8389b40263a9abc3
    To:       38683b0002f98ad0d8d0f00cc77ebdc48fc0de9e
    Nonce:    104
    GasPrice: 20000000000
    GasLimit  90000
    Value:    0
    Data:     0xf8a8fd6d
    V:        0x1c
    R:        0xcbf2fb79f27f5d0fcf1e404cc4f0ba5bcab7106a198416c7cdde0d336120aa04
    S:        0x1a6913b86ecb5b36bd233e386e93463049ddb9efd88b0eebbe0b20f0ab39efa1
    Hex:      f869688504a817c80083015f909438683b0002f98ad0d8d0f00cc77ebdc48fc0de9e8084f8a8fd6d1ca0cbf2fb79f27f5d0fcf1e404cc4f0ba5bcab7106a198416c7cdde0d336120aa04a01a6913b86ecb5b36bd233e386e93463049ddb9efd88b0eebbe0b20f0ab39efa1
)
}
I0808 20:59:55.969940 miner/worker.go:586] commit new work on block 9694 with 1 txs & 0 uncles. Took 913.702µs
worker.go case core.ChainHeadEvent. invoking commitNewWork
I0808 21:00:02.982379 miner/worker.go:348] 🔨  Mined block (#9694 / b38d8723). Wait 5 blocks for confirmation
### ^^ tx included in the second mined block
worker.go wait(). just mined a block, calling commitNewWork.

Steps to reproduce the behaviour

./go-ethereum/build/bin/geth --datadir "./local_testchain_data" --unlock "0xe9a4690662e26219d11315fe8389b40263a9abc3" --password "pass.conf" --rpc --rpccorsdomain "*" --networkid 1100 -maxpeers 0 --mine --minerthreads 1

Then send a tx, watch when the tx is included in the pending pool (eth.pendingTransactions in the geth console), and that an empty block is mined first, before the pending tx is included in the second block.

[karalabe]

I think this works as intended.

If a node is mining it will first collect all the executable transactions, and then start working on the PoW for those (if there are no transactions pending currently, it start working on an empty block's PoW). When already in this hashing phase it does not incorporate new transactions.

If it were to add every inbound transaction all the time, it would constantly need to stop the PoW hashing, generate a new block and restart the hashing. This would be quite a performance hit, especially if remote miners are connected which would need to poll or be notified of the update.

[cdetrio]

The PoW hashing should continue in parallel to block updates. If the pending block isn't updated until a new block is found (i.e. a ChainHeadEvent), then it adds ~15s of confirmation time to all transactions. If the overhead of pushing an updated block to workers is really a performance factor, then the updates could be throttled to once every X seconds.

It looks like this might also be the reason for the "uncle propagation" problem described here:

Propagating a block takes 2 seconds in Ethereum. Therefore miners should be including the uncle immediately in the following block. There must be a problem in Ethereum clients related to uncle propagation) [...]

That could be explained by ChainSideEvent only adding an uncle to the possibleUncles list, but not updating the pending block.

[fjl]

@cdetrio is right, it makes sense to construct the pending block continuously even while mining.
This line prevents it at the moment, but the change would be bigger than just removing the condition. The worker would need to change to maintain a 'next' Work in addition to the 'current' Work it keeps now.

[karalabe]

As far as I understood it, the request was not to maintain the "next" work,
but to constantly update the "current" one. Maybe I misunderstood?
On Aug 10, 2016 00:52, "Felix Lange" notifications@github.com wrote:

@cdetrio https://github.com/cdetrio is right, it makes sense to
construct the pending block continuously even while mining.
This line

go-ethereum/miner/worker.go

Line 237 in 5f55d95

if atomic.LoadInt32(&self.mining) == 0 {

prevents it
at the moment, but the change would be bigger than just removing the
condition. The worker would need to change to maintain a 'next' Work in
addition to the 'current' Work it keeps now.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#2897 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAH6GS6DfnQB7WPkNHW8VFYiP1_F-uaeks5qePaogaJpZM4JfplC
.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.