Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

Removes MAX_TX_TO_IMPORT from ChainSync #6976

Merged
merged 1 commit into from Nov 3, 2017
Merged

Removes MAX_TX_TO_IMPORT from ChainSync #6976

merged 1 commit into from Nov 3, 2017

Conversation

0x7CFE
Copy link
Contributor

@0x7CFE 0x7CFE commented Nov 3, 2017

This PR removes MAX_TX_TO_IMPORT constant and its uses in sync/src/chain.rs.

As @tomusdrw explained:

…back in the past we were parsing the transactions and actually importing them to the queue which took significant amount of time. Currently we have a queue of unverifiedTransactions that are later parsed and imported to the queue in batches (the method is now called queue_transactions). So the limiting doesn't currently make sense at all.

@0x7CFE 0x7CFE added M4-core ⛓ Core client code / Rust. A0-pleasereview 🤓 Pull request needs code review. labels Nov 3, 2017
Copy link
Collaborator

@tomusdrw tomusdrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Those transactions get queued in the client anyway and then processed in batches.
Currently we were discarding some transactions (if there was more than 512 in single packet)

@tomusdrw tomusdrw added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Nov 3, 2017
@arkpar arkpar merged commit 7eacef9 into master Nov 3, 2017
@arkpar arkpar deleted the fix_chain_limit branch November 3, 2017 12:12
@5chdn 5chdn added this to the 1.9 milestone Nov 4, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A8-looksgood 🦄 Pull request is reviewed well. M4-core ⛓ Core client code / Rust.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants