unique hashes for spc txs

[longwinterday]

No description provided.

[liam-lai]

Hi, can you fetch the latest code from dev-upgrade please? Your test is failed.

[wjrjerome]

Could you please create relevant unit or integration tests to cover the changes you made?

[wgr523]

I think you use this line time.Now().UnixNano() / int64(time.Millisecond) % txMatchGasLimit to avoid the transaction contains the same gas limit. However, there is still a small chance of 1/txMatchGasLimit such that the gas limit is the same. Have you considered using time.Now().UnixNano() / int64(time.Millisecond)?

[wgr523]

By the way, is it feasible to use incremental nonce, just like normal transactions? If nonce is incremented (nonce+=1), then nonce will be different for each transaction.

[longwinterday]

I think you use this line time.Now().UnixNano() / int64(time.Millisecond) % txMatchGasLimit to avoid the transaction contains the same gas limit. However, there is still a small chance of 1/txMatchGasLimit such that the gas limit is the same. Have you considered using time.Now().UnixNano() / int64(time.Millisecond)?

No. It is impossible to use Gas Limit larger when pool Gas Limit

XDPoSChain/core/tx_pool.go

Line 608 in be3e9eb

// Ensure the transaction doesn't exceed the current block limit gas.

However, if it be based on case that blocks appears every 2 seconds and it looks so, it is possible to use time.Now().UnixNano() / int64(time.Millisecond * 100) % txMatchGasLimit it is about 1/40_000_000 every 46days a chance to have same dynamicTxMatchGasLimit and the nonce should be kept but actually I can't find cases with triple and more not unique transactions. So the chances are about 1 not unique transaction every 105 000 years.

Nonce can be also incremented but it won't guarantee that every block will have special transaction cause of possible errors. Like get address from in that case

XDPoSChain/core/state_processor.go

Line 452 in 22479f1

from, err := types.Sender(types.MakeSigner(config, header.Number), tx)

[longwinterday]

Could you please create relevant unit or integration tests to cover the changes you made?

Hmm. I do not see special test cases for these changes. I suppose all current test cover the worker's functional.

[liam-lai]

can you please create a function in types that avoid duplicate code all over the place

[wjrjerome]

Could you please create relevant unit or integration tests to cover the changes you made?

Hmm. I do not see special test cases for these changes. I suppose all current test cover the worker's functional.

Ideally, it would be best to simulate the issue in unit test prior to your fix. i.e it shall fail.
Then run the test against your fix to verify it will pass.

[wjrjerome]

Have you considered creating a new function types.NewTransactionWithDynamicGasLimit where all the randomised logic such as uint64(time.Now().UnixNano() / int64(time.Millisecond) % txMatchGasLimit) can hide inside it.
Then call this function in this file instead of duplicating it across 4 different places?
Also, it would be very straightforward to write a unit test for such function.

[liam-lai]

@longwinterday could you reply above please

[liam-lai]

@longwinterday will this change create a hard forking?

[gzliudan]

solved by #430 #501 #521