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

Don't apply subjective billing to read-only transactions #114

Merged
merged 8 commits into from
Apr 20, 2022
2 changes: 1 addition & 1 deletion plugins/chain_plugin/chain_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2813,7 +2813,7 @@ void read_only::compute_transaction(const fc::variant_object& params, next_funct
abi_serializer::from_variant(params, *pretty_input, resolver, abi_serializer::create_yield_function( abi_serializer_max_time ));
} EOS_RETHROW_EXCEPTIONS(chain::packed_transaction_type_exception, "Invalid packed transaction")

app().get_method<incoming::methods::transaction_async>()(pretty_input, true, true,
app().get_method<incoming::methods::transaction_async>()(pretty_input, false, true,
[this, next](const std::variant<fc::exception_ptr, transaction_trace_ptr>& result) -> void {
if (std::holds_alternative<fc::exception_ptr>(result)) {
next(std::get<fc::exception_ptr>(result));
Expand Down
10 changes: 7 additions & 3 deletions plugins/producer_plugin/producer_plugin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,8 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin

bool disable_subjective_billing = ( _pending_block_mode == pending_block_mode::producing )
|| ( persist_until_expired && _disable_subjective_api_billing )
|| ( !persist_until_expired && _disable_subjective_p2p_billing );
|| ( !persist_until_expired && _disable_subjective_p2p_billing )
|| trx->read_only;
heifner marked this conversation as resolved.
Show resolved Hide resolved

auto first_auth = trx->packed_trx()->get_transaction().first_authorizer();
uint32_t sub_bill = 0;
Expand All @@ -584,7 +585,9 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
}
exhausted = block_is_exhausted();
} else {
_subjective_billing.subjective_bill_failure( first_auth, trace->elapsed, fc::time_point::now() );
if (!disable_subjective_billing)
_subjective_billing.subjective_bill_failure( first_auth, trace->elapsed, fc::time_point::now() );
heifner marked this conversation as resolved.
Show resolved Hide resolved

auto e_ptr = trace->except->dynamic_copy_exception();
send_response( e_ptr );
}
heifner marked this conversation as resolved.
Show resolved Hide resolved
Expand All @@ -596,7 +599,8 @@ class producer_plugin_impl : public std::enable_shared_from_this<producer_plugin
_unapplied_transactions.add_persisted( trx );
} else {
// if db_read_mode SPECULATIVE then trx is in the pending block and not immediately reverted
_subjective_billing.subjective_bill( trx->id(), expire, first_auth, trace->elapsed,
if (!disable_subjective_billing)
_subjective_billing.subjective_bill( trx->id(), expire, first_auth, trace->elapsed,
chain.get_read_mode() == chain::db_read_mode::SPECULATIVE );
}
send_response( trace );
Expand Down
2 changes: 1 addition & 1 deletion tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ add_test(NAME producer-preactivate-feature-test COMMAND tests/prod_preactivation
set_property(TEST producer-preactivate-feature-test PROPERTY LABELS nonparallelizable_tests)
add_test(NAME nodeos_protocol_feature_test COMMAND tests/nodeos_protocol_feature_test.py -v --clean-run --dump-error-detail WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST nodeos_protocol_feature_test PROPERTY LABELS nonparallelizable_tests)
add_test(NAME compute_transaction_test COMMAND tests/compute_transaction_test.py -v -p 2 --clean-run --dump-error-detail WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
add_test(NAME compute_transaction_test COMMAND tests/compute_transaction_test.py -v -p 2 -n 3 --clean-run --dump-error-detail WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
set_property(TEST compute_transaction_test PROPERTY LABELS nonparallelizable_tests)

add_test(NAME distributed-transactions-test COMMAND tests/distributed-transactions-test.py -d 2 -p 4 -n 6 -v --clean-run --dump-error-detail WORKING_DIRECTORY ${CMAKE_BINARY_DIR})
Expand Down
5 changes: 5 additions & 0 deletions tests/Node.py
Original file line number Diff line number Diff line change
Expand Up @@ -851,6 +851,11 @@ def getEosBalances(self, accounts):

return balances

# Gets subjective bill info for an account
def getAccountSubjectiveInfo(self, account):
acct = self.getEosAccount(account)
return acct["subjective_cpu_bill_limit"]
heifner marked this conversation as resolved.
Show resolved Hide resolved

# Gets accounts mapped to key. Returns json object
def getAccountsByKey(self, key, exitOnError=False):
cmdDesc = "get accounts"
Expand Down
48 changes: 45 additions & 3 deletions tests/compute_transaction_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@
Print ("producing nodes: %s, non-producing nodes: %d, topology: %s, delay between nodes launch(seconds): %d" % (pnodes, total_nodes-pnodes, topo, delay))

Print("Stand up cluster")
if cluster.launch(pnodes=pnodes, totalNodes=total_nodes, topo=topo, delay=delay) is False:
if cluster.launch(pnodes=pnodes, totalNodes=total_nodes, topo=topo, delay=delay,extraNodeosArgs=" --http-max-response-time-ms 990000 --disable-subjective-api-billing false" ) is False:
errorExit("Failed to stand up eos cluster.")

Print ("Wait for Cluster stabilization")
Expand All @@ -91,12 +91,17 @@
account2 = Account('account2')
account2.ownerPublicKey = EOSIO_ACCT_PUBLIC_DEFAULT_KEY
account2.activePublicKey = EOSIO_ACCT_PUBLIC_DEFAULT_KEY
cluster.createAccountAndVerify(account2, cluster.eosioAccount, stakedDeposit=1000)
cluster.createAccountAndVerify(account2, cluster.eosioAccount, stakedDeposit=1000, stakeCPU=1)

Print("Validating accounts after bootstrap")
cluster.validateAccounts([account1, account2])

node = cluster.getNode()

# non-producing node
npnode = cluster.nodes[-1]


transferAmount="1000.0000 {0}".format(CORE_SYMBOL)

node.transferFunds(cluster.eosioAccount, account1, transferAmount, "fund account")
Expand Down Expand Up @@ -125,8 +130,45 @@

postBalances = node.getEosBalances([account1, account2])

testSuccessful = (postBalances == preBalances)
assert(postBalances == preBalances)

# Send through some failing *read only* transactions on a non-producer node
for x in range(5):
memo = 'tx-{}'.format(x)
trx2 = {

"actions": [{"account": "eosio.token","name": "transfer",
"authorization": [{"actor": "account2","permission": "active"}],
"data": {"from": "account2","to": "account1","quantity": "10.0001 SYS","memo": memo},
"compression": "none"}]
}

results = npnode.pushTransaction(trx2, opts="--read-only")
assert(not results[0])

# Verify that no subjective billing was charged
acct2 = npnode.getAccountSubjectiveInfo("account2")
assert(acct2["used"] == 0)

# Send through some failing *non read-only* transactions on a non-producer node
for x in range(5):
memo = 'tx-{}'.format(x)
trx2 = {

"actions": [{"account": "eosio.token","name": "transfer",
"authorization": [{"actor": "account2","permission": "active"}],
"data": {"from": "account2","to": "account1","quantity": "10.0001 SYS","memo": memo},
"compression": "none"}]
}

results = npnode.pushTransaction(trx2)
assert(not results[0])

# Verify that subjective billing *was* charged
acct2 = npnode.getAccountSubjectiveInfo("account2")
assert(acct2["used"] > 0)

testSuccessful = True
finally:
TestHelper.shutdown(cluster, walletMgr, testSuccessful, killEosInstances, killWallet, keepLogs, killAll, dumpErrorDetails)

Expand Down