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

remove block view! #9397

Merged
merged 1 commit into from
Aug 24, 2018
Merged

remove block view! #9397

merged 1 commit into from
Aug 24, 2018

Conversation

debris
Copy link
Collaborator

@debris debris commented Aug 22, 2018

No description provided.

@debris debris added A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. M4-core ⛓ Core client code / Rust. A0-pleasereview 🤓 Pull request needs code review. and removed A3-inprogress ⏳ Pull request is in progress. No review needed at this stage. labels Aug 22, 2018
@debris
Copy link
Collaborator Author

debris commented Aug 23, 2018

I decided to remove view! in several chunk as this pr would get to big to review it properly.

@dvdplm
Copy link
Collaborator

dvdplm commented Aug 23, 2018

Can you tell us why you want to remove it? What's wrong with it?

@debris
Copy link
Collaborator Author

debris commented Aug 23, 2018

it unsafely unwrap rlp. We have a bunch of issues related to that

@5chdn 5chdn added this to the 2.1 milestone Aug 23, 2018

/// Enact the block given by `block_bytes` using `engine` on the database `db` with given `parent` block header
fn enact_bytes(
block_bytes: &[u8],
block_bytes: Vec<u8>,
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain the motivation behind using a Vec instead of a Slice here?

Vec is less efficient (syscall overhead) and consumes more memory (3 words instead of 2)?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like test functions

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah lol thanks, never mind 😃

@ascjones ascjones merged commit b87c7ca into master Aug 24, 2018
@ascjones ascjones deleted the remove_block_view branch August 24, 2018 09:53
@5chdn 5chdn added A8-looksgood 🦄 Pull request is reviewed well. and removed A0-pleasereview 🤓 Pull request needs code review. labels Aug 24, 2018
dvdplm added a commit that referenced this pull request Aug 30, 2018
* master:
  evmbin: Fix gas_used issue in state root mismatch and handle output better (#9418)
  Update hardcoded sync (#9421)
  Add block reward contract config to ethash and allow off-chain contracts (#9312)
  Private packets verification and queue refactoring (#8715)
  Update tobalaba.json (#9419)
  docs: add parity ethereum logo to readme (#9415)
  build: update rocksdb crate (#9414)
  Updating the CI system  (#8765)
  Better support for eth_getLogs in light mode (#9186)
  Add update docs script to CI (#9219)
  `gasleft` extern implemented for WASM runtime (kip-6) (#9357)
  block view! removal in progress (#9397)
  Prevent sync restart if import queue full (#9381)
  nonroot CentOS Docker image (#9280)
  ethcore: kovan: delay activation of strict score validation (#9406)
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.

5 participants