Skip to content

Remove TxTimlock from the mempool#12

Closed
remagpie wants to merge 4 commits intoCodeChain-io:masterfrom
remagpie:no-miner-timelock
Closed

Remove TxTimlock from the mempool#12
remagpie wants to merge 4 commits intoCodeChain-io:masterfrom
remagpie:no-miner-timelock

Conversation

@remagpie
Copy link
Contributor

Resolves #8
I think there might be some parts that can be further simplified, but I'm not sure about them for now.

@remagpie remagpie requested a review from sgkim126 December 19, 2019 11:20
|| next_seq < current_seq
{
self.check_transactions(public, current_seq, inserted_block_number, inserted_timestamp)
self.check_transactions(public, current_seq)
Copy link
Contributor

Choose a reason for hiding this comment

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

let target_seq = if current_seq < first_seq
    || inserted_block_number < self.last_block_number
    || inserted_timestamp < self.last_timestamp
    || inserted_timestamp < self.last_timestamp
    || next_seq < current_seq {
    current_seq
} else {
    next_seq
};
let new_next_seq = self.check_transactions(public, target_seq);

current_block_number: PoolingInstant,
current_timestamp: u64,
) -> u64 {
fn check_transactions(&self, public: Public, start_seq: u64) -> u64 {
Copy link
Contributor

Choose a reason for hiding this comment

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

Why does this function still remain?
It seems that this function was introduced because of a timelock.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think this function can be removed, but I couldn't since I didn't fully understand the mempool implementation yet.
It's returning some value derived from start_seq, and I'm not sure what it means.

let id = self.next_transaction_id;
self.next_transaction_id += 1;
let item = MemPoolItem::new(tx, origin, inserted_block_number, inserted_timestamp, id, timelock);
let item = MemPoolItem::new(tx, origin, inserted_block_number, inserted_timestamp, id, TxTimelock {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think rebasing this PR on #11 will make this commit clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll try it when #11 is merged.

@remagpie remagpie force-pushed the no-miner-timelock branch 3 times, most recently from f552920 to 2fac811 Compare December 24, 2019 01:56
@remagpie
Copy link
Contributor Author

Rebased to the latest master.

@majecty
Copy link

majecty commented Jan 29, 2020

I created a new PR #120 with the same commits as this PR. I rebased these commits in the master branch.

@majecty majecty closed this Jan 29, 2020
@remagpie remagpie deleted the no-miner-timelock branch January 29, 2020 18:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simplify mempool

3 participants