Skip to content

Extract should_replace from Scoring and supply pooled txs from same sender#116

Merged
ascjones merged 12 commits into
masterfrom
aj-scoring-readiness
Mar 28, 2019
Merged

Extract should_replace from Scoring and supply pooled txs from same sender#116
ascjones merged 12 commits into
masterfrom
aj-scoring-readiness

Conversation

@ascjones
Copy link
Copy Markdown
Contributor

@ascjones ascjones commented Mar 21, 2019

should_replace does not use the computed score, and is actually called only on import (when pool limits are reached).

Further, we pass other pooled transactions from the respective senders of the old and new transactions which are being compared. This would allow an implementation to also compare the Readiness of the transactions when deciding if the new should push out the old.

Copy link
Copy Markdown
Contributor

@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.

lgtm! We should bump the version of the crate though.

Comment thread transaction-pool/src/replace.rs Outdated
@ascjones
Copy link
Copy Markdown
Contributor Author

Bump to version 2.0?

Comment thread transaction-pool/src/pool.rs Outdated
@tomusdrw
Copy link
Copy Markdown
Contributor

Seems like there is still an issue with tests:

error[E0053]: method `should_replace` has an incompatible type for trait
  --> transaction-pool/src/tests/helpers.rs:77:20
   |
77 |     fn should_replace(&mut self, old: &ReplaceTransaction<Transaction>, new: &ReplaceTransaction<Transaction>) -> scoring::Choice {
   |                       ^^^^^^^^^ types differ in mutability
   | 
  ::: transaction-pool/src/replace.rs:52:23
   |
52 |     fn should_replace(&self, old: &ReplaceTransaction<T>, new: &ReplaceTransaction<T>) -> Choice;
   |                       ----- type in trait
   |
   = note: expected type `fn(&tests::helpers::DummyScoring, &replace::ReplaceTransaction<'_, tests::Transaction>, &replace::ReplaceTransaction<'_, tests::Transaction>) -> scoring::Choice`
              found type `fn(&mut tests::helpers::DummyScoring, &replace::ReplaceTransaction<'_, tests::Transaction>, &replace::ReplaceTransaction<'_, tests::Transaction>) -> scoring::Choice`
help: consider change the type to match the mutability in trait
   |
77 |     fn should_replace(&self, old: &ReplaceTransaction<Transaction>, new: &ReplaceTransaction<Transaction>) -> scoring::Choice {
   |                       ^^^^^

@tomusdrw
Copy link
Copy Markdown
Contributor

Merging since appveyor failures are unrelated and travis only failed the windows build (due to timeout).

@ascjones ascjones merged commit 45b4fae into master Mar 28, 2019
@ascjones ascjones deleted the aj-scoring-readiness branch March 28, 2019 13:24
ordian added a commit that referenced this pull request Apr 4, 2019
* master:
  change dependencies of `keccak-hash`and `trie-standardmap` (#111)
  bring appveyor back (#118)
  Extract `should_replace` from Scoring and supply pooled txs from same sender (#116)
ordian added a commit that referenced this pull request May 6, 2019
* master:
  Bump fixed-hash to 0.3.2 (#134)
  Use EntropyRng instead of OsRng (#130)
  Bump parity-crypto to 0.3.1 (#132)
  rust-crypto dependency removal (#85)
  Use newly stablized TryFrom trait for primitive-types (#127)
  change dependencies of `keccak-hash`and `trie-standardmap` (#111)
  bring appveyor back (#118)
  Extract `should_replace` from Scoring and supply pooled txs from same sender (#116)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants