Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bounded active transaction #1990

Merged
merged 16 commits into from May 20, 2019

Conversation

@argakiig
Copy link
Collaborator

commented May 16, 2019

This PR serves to bound the active transactions container dynamically based on the number of transactions that are over the long_announcement threshold and the number of confirmed blocks per second. If the roots container is over 75,50,25% saturated with long_announcements it will drop the two lowest adjusted_difficulty(accounting for block ordering during confirmation and insertion) blocks that are not managed/monitored by the work_watcher that are also long_unconfirmed

argakiig added some commits May 13, 2019

move tests to active_transactions
add quick transaction_counter
validate transaction_counter
flush active_transactions based on transaction_counter.rate
counter incremented on active_transactions add() successful
if counter.rate == 0 set minimum_size low
fix error in trend_sample
tests to flush lowest difficulty blocks
formatting
cleanup

@argakiig argakiig added this to the V19.0 milestone May 16, 2019

@argakiig argakiig self-assigned this May 16, 2019

Show resolved Hide resolved nano/core_test/active_transactions.cpp
Show resolved Hide resolved nano/core_test/active_transactions.cpp Outdated
Show resolved Hide resolved nano/core_test/active_transactions.cpp Outdated
Show resolved Hide resolved nano/node/active_transactions.cpp
Show resolved Hide resolved nano/node/active_transactions.cpp Outdated

@zhyatt zhyatt added this to During RC in V19 May 16, 2019

@cryptocode
Copy link
Collaborator

left a comment

LGTM pending comments

Show resolved Hide resolved nano/node/active_transactions.cpp Outdated
Show resolved Hide resolved nano/node/active_transactions.cpp Outdated
Show resolved Hide resolved nano/node/wallet.cpp Outdated

argakiig added some commits May 17, 2019

@argakiig argakiig requested review from wezrule, SergiySW and cryptocode May 17, 2019

class transaction_counter final
{
public:
transaction_counter ();

This comment has been minimized.

Copy link
@wezrule

wezrule May 17, 2019

Collaborator

No need to supply an empty default constructor here

This comment has been minimized.

Copy link
@argakiig

argakiig May 17, 2019

Author Collaborator

thanks, forgot i removed everything from this :)

argakiig added some commits May 17, 2019

remove empty constructor
always trigger a flush if roots.size > 100k

argakiig added some commits May 18, 2019

@argakiig argakiig merged commit 264842e into nanocurrency:master May 20, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@zhyatt zhyatt moved this from During RC to RC 3 (TBD) in V19 May 20, 2019

argakiig added a commit to argakiig/raiblocks that referenced this pull request May 22, 2019

Bounded active transaction (nanocurrency#1990)
* check if block is being watched by the wallet

* iterate over roots to return count of long unconfirmed blocks

* move tests to active_transactions
add quick transaction_counter
validate transaction_counter

* Validate adjusted_difficulty ordering of roots container

* flush active_transactions based on transaction_counter.rate
counter incremented on active_transactions add() successful
if counter.rate == 0 set minimum_size low
fix error in trend_sample
tests to flush lowest difficulty blocks

* formatting

* formatting
cleanup

* feedback from reviews
fix ub in flush_lowest

* missed superfluous unlock during clean up

* use return value from erase

* remove empty constructor
always trigger a flush if roots.size > 100k

* use getter instead of lock

* fix transaction_counter test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.