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

Reorder transaction_by_hash to favour canon search #2332

Merged
merged 4 commits into from
Sep 27, 2016

Conversation

gavofyork
Copy link
Contributor

No description provided.

@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. B0-patch labels Sep 26, 2016
@gavofyork gavofyork added the M4-core ⛓ Core client code / Rust. label Sep 26, 2016
.or_else(|| miner.transaction(&hash).map(Transaction::from));
match maybe_tx {
Some(t) => to_value(&t),
None => Ok(Value::Null),
Copy link
Collaborator

Choose a reason for hiding this comment

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

rob recently changed function signatures. This code won't compile.

shorter version of this function:

let miner = take_weak!(self.miner);
match try!(self.transaction(&hash)) {
  None => Ok(miner.transaction(&hash).map(Into::into))
  tx => tx
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

not quite (hash is still RpcH256, not H256). latest patch fixes.

@debris debris added A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 26, 2016
@gavofyork gavofyork added A0-pleasereview 🤓 Pull request needs code review. and removed A4-gotissues 💥 Pull request is reviewed and has significant issues which must be addressed. labels Sep 26, 2016
@gavofyork gavofyork modified the milestone: 1.3.2 Sep 26, 2016
@debris debris added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Sep 26, 2016
@coveralls
Copy link

Coverage Status

Coverage increased (+0.06%) to 85.235% when pulling dabf6de on fix-transaction-by-hash into 92451ef on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.07%) to 85.243% when pulling be3c271 on fix-transaction-by-hash into 92451ef on master.

@gavofyork gavofyork merged commit 3fb3f1f into master Sep 27, 2016
@gavofyork gavofyork deleted the fix-transaction-by-hash branch September 27, 2016 10:17
@gavofyork gavofyork removed this from the 1.3.2 milestone Sep 27, 2016
jacogr added a commit that referenced this pull request Sep 29, 2016
* master:
  Fixing Delegate Call in JIT (#2378)
  Prioritizing re-imported transactions (#2372)
  Revert #2172, pretty much. (#2387)
  correct sync memory usage calculation (#2385)
  Update gitlab-ci
  Fix the traceAddress field in transaction traces. (#2373)
  Removing extras data from retracted blocks. (#2375)
  fixed #2263, geth keys with ciphertext shorter than 32 bytes (#2318)
  Expanse compatibility (#2369)
  Specify column cache sizes explicitly; default fallback of 2MB (#2358)
  Canonical state cache (master) (#2311)
  make block queue into a more generic verification queue and fix block heap size calculation (#2095)
  Hash Content RPC method (#2355)
  Reorder transaction_by_hash to favour canon search (#2332)
  DIV optimization (#2327)
  Error when deserializing invalid hex (#2339)
  Changed http:// to https:// on some links (#2349)
  add a test
  fix migration system, better errors

# Conflicts:
#	.gitlab-ci.yml
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.

3 participants