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.

Sign up for GitHub

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

Jump to bottom

Change ordering of TXs by time #1613

Merged
merged 6 commits into from
Jan 3, 2023
Merged

Change ordering of TXs by time #1613

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
1a75489
Loop TXs by time
Bushstar Nov 28, 2022
7b394a9
Merge branch 'master' into miner-loop-on-time
Bushstar Nov 28, 2022
b6e93b3
Add flag to enable block ordering by fee
Bushstar Nov 29, 2022
15f27e8
tests: update circular deps. run test with tx fee order.
Bushstar Nov 30, 2022
e8e430f
tests: fix intermittent fault
Bushstar Dec 1, 2022
dcc9100
Default to fee ordering
Bushstar Dec 6, 2022
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
2 changes: 1 addition & 1 deletion src/init.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,7 @@ void SetupServerArgs()
gArgs.AddArg("-regtest-minttoken-simulate-mainnet", "Simulate mainnet for minttokens on regtest - default behavior on regtest is to allow anyone to mint mintable tokens for ease of testing", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
gArgs.AddArg("-simulatemainnet", "Configure the regtest network to mainnet target timespan and spacing ", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::OPTIONS);
gArgs.AddArg("-dexstats", strprintf("Enable storing live dex data in DB (default: %u)", DEFAULT_DEXSTATS), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
gArgs.AddArg("-blockfeeordering", strprintf("Whether to order transactions by fee, otherwise ordered by time (default: %u)", DEFAULT_FEE_ORDERING), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
gArgs.AddArg("-blocktimeordering", strprintf("Whether to order transactions by time, otherwise ordered by fee (default: %u)", DEFAULT_FEE_ORDERING), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why use fee as default? I thought the default should be time to prevent frontrunning. @prasannavl

Copy link
Member

@prasannavl prasannavl Jan 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, I missed this earlier. It's because fee is quite an intricate part of quite a few things, including fee estimation and many things that rely on this behaviour.

In order to avoid surprises, and also since this is a new behaviour, would be best to let it be used optionally for a while, before considering it as default.

The front-running aspects will be prevented as more and more MNs start using this, which we can expect as more and more community starts opting into this behaviour.

#ifdef USE_UPNP
#if USE_UPNP
gArgs.AddArg("-upnp", "Use UPnP to map the listening port (default: 1 when listening and no -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION);
Expand Down
8 changes: 4 additions & 4 deletions src/miner.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,11 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock(const CScript& sc
UpdateTime(pblock, consensus, pindexPrev); // update time before tx packaging
}

const auto feeOrdering = gArgs.GetBoolArg("-blockfeeordering", DEFAULT_FEE_ORDERING);
if (feeOrdering) {
addPackageTxs<ancestor_score>(nPackagesSelected, nDescendantsUpdated, nHeight, mnview);
} else {
const auto timeOrdering = gArgs.GetBoolArg("-blocktimeordering", DEFAULT_FEE_ORDERING);
if (timeOrdering) {
addPackageTxs<entry_time>(nPackagesSelected, nDescendantsUpdated, nHeight, mnview);
} else {
addPackageTxs<ancestor_score>(nPackagesSelected, nDescendantsUpdated, nHeight, mnview);
}


Expand Down
4 changes: 2 additions & 2 deletions src/policy/fees.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -788,8 +788,8 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, FeeCalculation
{
LOCK(m_cs_fee_estimator);

// If block ordering by fee is not enabled return 0 to let fallback or discard fee be used.
if (!gArgs.GetBoolArg("-blockfeeordering", DEFAULT_FEE_ORDERING)) {
// If block ordering by time is enabled return 0 to let fallback or discard fee be used.
if (gArgs.GetBoolArg("-blocktimeordering", DEFAULT_FEE_ORDERING)) {
return 0;
}

Expand Down
2 changes: 1 addition & 1 deletion src/rpc/mining.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -979,7 +979,7 @@ static UniValue estimatesmartfee(const JSONRPCRequest& request)
CFeeRate feeRate = ::feeEstimator.estimateSmartFee(conf_target, &feeCalc, conservative);
if (feeRate != CFeeRate(0)) {
result.pushKV("feerate", ValueFromAmount(feeRate.GetFeePerK()));
} else if (!gArgs.GetBoolArg("-blockfeeordering", DEFAULT_FEE_ORDERING)) {
} else if (gArgs.GetBoolArg("-blocktimeordering", DEFAULT_FEE_ORDERING)) {
result.pushKV("feerate", ValueFromAmount(DEFAULT_TRANSACTION_MINFEE));
} else {
errors.push_back("Insufficient data or no feerate found");
Expand Down
6 changes: 3 additions & 3 deletions test/functional/feature_fee_estimation.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,9 +130,9 @@ def set_test_params(self):
# mine non-standard txs (e.g. txs with "dust" outputs)
# Force fSendTrickle to true (via whitelist)
self.extra_args = [
["-acceptnonstdtxn", "-whitelist=127.0.0.1", "-blockfeeordering=1"],
["-acceptnonstdtxn", "-whitelist=127.0.0.1", "-blockmaxweight=68000", "-blockfeeordering=1"],
["-acceptnonstdtxn", "-whitelist=127.0.0.1", "-blockmaxweight=32000", "-blockfeeordering=1"],
["-acceptnonstdtxn", "-whitelist=127.0.0.1"],
["-acceptnonstdtxn", "-whitelist=127.0.0.1", "-blockmaxweight=68000"],
["-acceptnonstdtxn", "-whitelist=127.0.0.1", "-blockmaxweight=32000"],
]

def skip_test_if_missing_module(self):
Expand Down
1 change: 0 additions & 1 deletion test/functional/mining_prioritisetransaction.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ def set_test_params(self):
self.extra_args = [[
"-printpriority=1",
"-acceptnonstdtxn=1",
"-blockfeeordering=1",
]] * self.num_nodes

def skip_test_if_missing_module(self):
Expand Down