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

Fix parallel transactions race-condition #10995

Merged
merged 20 commits into from
Sep 11, 2019
Merged

Fix parallel transactions race-condition #10995

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
48070be
WiP : clear pending txs cache & tick in Miner
ngotchac Aug 15, 2019
e9c110f
Fixed pending transactions
ngotchac Aug 16, 2019
cc653a0
Revert debugging code
ngotchac Aug 26, 2019
55772a0
Add ToDo comment
ngotchac Aug 26, 2019
75921ed
Remove commented-out code
ngotchac Aug 27, 2019
897eef0
Reverse LTO setting
ngotchac Aug 27, 2019
7b8807e
Merge branch 'master' into ng-tx-queue-fix
ngotchac Aug 27, 2019
9abf8ac
WiP
ngotchac Sep 2, 2019
2d0c706
Merge branch 'master' into ng-tx-queue-fix
ngotchac Sep 2, 2019
6980320
Try to seal a new block if there are pending transactions
ngotchac Sep 4, 2019
8080777
Try resealing only for internal imports
ngotchac Sep 4, 2019
6b5c31d
Remove logging
ngotchac Sep 4, 2019
7004b54
Use AtomicU64 instead of Mutex<BlockNumber>
ngotchac Sep 4, 2019
33e5809
Remove TxQueue cache clear // Update AtomicUint logic
ngotchac Sep 10, 2019
eea5d32
Update comments in Miner
ngotchac Sep 10, 2019
40e243a
Revert import of `parking_lot`
ngotchac Sep 10, 2019
b633cb8
Merge branch 'master' into ng-tx-queue-fix
ngotchac Sep 10, 2019
47c102b
Update `transaction-pool` dependency
ngotchac Sep 11, 2019
159cc84
Call directly `update_sealing`
ngotchac Sep 11, 2019
51c188d
Call `update_sealing` directly
ngotchac Sep 11, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 6 additions & 8 deletions ethcore/engines/instant-seal/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Parity Ethereum. If not, see <http://www.gnu.org/licenses/>.

use std::collections::HashSet;
use std::sync::atomic::{AtomicU64, Ordering};

use common_types::{
header::Header,
Expand All @@ -24,15 +24,13 @@ use common_types::{
params::CommonParams,
},
errors::EthcoreError as Error,
BlockNumber,
};
use engine::Engine;
use ethjson;
use machine::{
ExecutedBlock,
Machine
};
use parking_lot::Mutex;


/// `InstantSeal` params.
Expand All @@ -55,7 +53,7 @@ impl From<ethjson::spec::InstantSealParams> for InstantSealParams {
pub struct InstantSeal {
params: InstantSealParams,
machine: Machine,
sealed_blocks: Mutex<HashSet<BlockNumber>>,
last_sealed_block: AtomicU64,
}

impl InstantSeal {
Expand All @@ -64,7 +62,7 @@ impl InstantSeal {
InstantSeal {
params,
machine,
sealed_blocks: Mutex::new(HashSet::new()),
last_sealed_block: AtomicU64::new(0),
}
}
}
Expand All @@ -80,10 +78,10 @@ impl Engine for InstantSeal {
if block.transactions.is_empty() {
Seal::None
} else {
let mut sealed_blocks = self.sealed_blocks.lock();
let block_number = block.header.number();
if !sealed_blocks.contains(&block_number) {
sealed_blocks.insert(block_number);
let last_sealed_block = self.last_sealed_block.load(Ordering::SeqCst);
if block_number > last_sealed_block {
self.last_sealed_block.store(block_number, Ordering::SeqCst);
ngotchac marked this conversation as resolved.
Show resolved Hide resolved
Seal::Regular(Vec::new())
} else {
Seal::None
Expand Down
5 changes: 3 additions & 2 deletions ethcore/src/miner/miner.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -880,8 +880,9 @@ impl Miner {

/// Call `update_sealing` if needed
dvdplm marked this conversation as resolved.
Show resolved Hide resolved
pub fn maybe_update_sealing<C: miner::BlockChainClient>(&self, chain: &C) {
use miner::MinerService;
ngotchac marked this conversation as resolved.
Show resolved Hide resolved
if self.transaction_queue.has_local_pending_transactions() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure about this. The implementation is very specific for NullEngine (i.e. checks local transactions), while we have plenty of other conditions that affect when to seal. Why can't we just call update_sealing instead?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is still valid.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah sorry, I thought you were talking about the next line, where I first used prepare_and_update_sealing. I just updated it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I guess then there is no point in adding maybe_update_sealing to the traits and just call update_sealing directly?

self.prepare_and_update_sealing(chain);
self.update_sealing(chain);
}
}
}
Expand Down Expand Up @@ -1250,7 +1251,7 @@ impl miner::MinerService for Miner {
/// Update sealing if required.
/// Prepare the block and work if the Engine does not seal internally.
fn update_sealing<C>(&self, chain: &C) where
C: miner::BlockChainClient,
C: BlockChain + CallContract + BlockProducer + SealedBlockImporter + Nonce + Sync,
{
trace!(target: "miner", "update_sealing");

Expand Down
2 changes: 1 addition & 1 deletion ethcore/src/miner/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ pub trait MinerService : Send + Sync {

/// Update current pending block
fn update_sealing<C>(&self, chain: &C)
where C: BlockChainClient;
where C: BlockChain + CallContract + BlockProducer + SealedBlockImporter + Nonce + Sync;

// Notifications

Expand Down