EthMiner crate #653
EthMiner crate #653
Conversation
Conflicts: parity/main.rs
Gas price threshold for transactions
Conflicts: Cargo.lock Cargo.toml hook.sh miner/src/transaction_queue.rs rpc/Cargo.toml sync/Cargo.toml sync/src/chain.rs
gavofyork
added
A1-onice 🌨
Conflicts: sync/src/lib.rs
|
presumably this kind of reverses/rewrites #641, no? |
Conflicts: ethcore/src/client.rs parity/main.rs rpc/src/v1/impls/eth.rs sync/src/chain.rs
tomusdrw
removed
the
A7-looksgoodcantmerge 🙄
Conflicts: rpc/src/v1/impls/eth.rs
Conflicts: util/bigint/src/uint.rs
Conflicts: ethcore/src/client/client.rs parity/main.rs sync/src/chain.rs sync/src/lib.rs
Conflicts: miner/src/miner.rs parity/main.rs
RPC Pending Transactions Filter
Conflicts: Cargo.toml rpc/Cargo.toml sync/Cargo.toml
gavofyork
added
A0-pleasereview 🤓
Conflicts: parity/main.rs
| } | ||
|
|
||
| /// New chain head event. Restart mining operation. | ||
| pub fn prepare_sealing(&self) { | ||
| fn prepare_sealing(&self, author: Address, extra_data: Bytes, transactions: Vec<SignedTransaction>) -> Option<ClosedBlock> { |
There was a problem hiding this comment.
will be nice to move this function into the ethminer crate eventually - that can happen in civility though.
|
looks like this has some of the other recent changes in. can we get a proper diff to review? |
|
It contains #661 and #640 which has been reviewed and merged to this branch. This PR only introduces ethminer crate and includes transactions to mined block. I can split it into couple of PRs if you prefer (but it basicly requires reverting commits and reverting the reverts in separate prs) |
|
ahh i see. that rather makes it hard to see what changes have been reviewed and what are new :-/ |
|
|
||
| { | ||
| let good = good.par_iter().map(|h| fetch_transactions(chain, h)); | ||
| let bad = bad.par_iter().map(|h| fetch_transactions(chain, h)); |
There was a problem hiding this comment.
is this really "bad" (i.e. invalid)? or just retracted? if retracted, then rest of logic is fine. if invalid, then rest of logic is broken (we don't want to be adding transactions from invalid blocks).
|
1000+ lines changed over 32 files. this is too big, and more importantly, diffuse. the fact that some arbitrary set of those changes have already been reviewed doesn't help a bit, since we can't tell which parts. this includes various refactorings, some small, some big. i've reviewed it as best as i can and the style and basic premise is good, but there is no way i would sign off on the logic or there being no unintended consequences. would have been much nicer if we had incremental alterations where each step can be logic-checked. |
gavofyork
added
A6-mustntgrumble 💦
Same as #632 but to master
Please merge #640 & #650 & #651 first.