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

Skip transaction size check while replaying #1716

Merged
merged 1 commit into from Apr 12, 2019

Conversation

Projects
3 participants
@jmjatlanta
Copy link
Contributor

commented Apr 11, 2019

Checking the transaction size was added in v3.0.0, but is not needed when replaying. This PR skips the transaction size check while replaying.

Fixes #1619

@jmjatlanta jmjatlanta merged commit 5563cd4 into develop Apr 12, 2019

2 of 3 checks passed

continuous-integration/travis-ci/push The Travis CI build is in progress
Details
ci/dockercloud Your tests passed in Docker Cloud
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@jmjatlanta jmjatlanta deleted the jmj_1619 branch Apr 12, 2019

@jmjatlanta jmjatlanta referenced this pull request Apr 12, 2019

Closed

Skip transaction size check during replay #1619

1 of 17 tasks complete
FC_ASSERT( head_block_time() <= HARDFORK_CORE_1573_TIME
|| trx.get_packed_size() <= chain_parameters.maximum_transaction_size,
"Transaction exceeds maximum transaction size." );
if ( !(skip & skip_block_size_check ) ) // don't waste time on replay

This comment has been minimized.

Copy link
@abitmore

abitmore Apr 12, 2019

Member

IMHO this code is wrong. skip_block_size_check flag is set when pushing pending transactions, that means the large transactions will propagate and will accumulate in RAM.

This comment has been minimized.

Copy link
@jmjatlanta

jmjatlanta Apr 12, 2019

Author Contributor

Ooff! I will test and attempt a different solution. Thanks for the catch.

This comment has been minimized.

Copy link
@abitmore

abitmore Apr 12, 2019

Member

OK, I was wrong, was misled by the comment here:

/**
* Attempts to push the transaction into the pending queue
*
* When called to push a locally generated transaction, set the skip_block_size_check bit on the skip argument. This
* will allow the transaction to be pushed even if it causes the pending block size to exceed the maximum block size.
* Although the transaction will probably not propagate further now, as the peers are likely to have their pending
* queues full as well, it will be kept in the queue to be propagated later when a new block flushes out the pending
* queues.
*/
processed_transaction database::push_transaction( const precomputable_transaction& trx, uint32_t skip )

Actually we didn't set the flag when pushing transactions to pending queue:

_chain_db->push_transaction( transaction_message.trx );

_app.chain_database()->push_transaction(trx);

_app.chain_database()->push_transaction(trx);

push_transaction is called with default skip parameter, aka skip_nothing.

@abitmore abitmore added this to In development in Feature Release (3.1.0) via automation Apr 12, 2019

@abitmore abitmore added this to the 3.1.0 - Feature Release milestone Apr 12, 2019

@abitmore abitmore moved this from In development to Done in Feature Release (3.1.0) Apr 12, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.