Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Rel 2.0.x: Subjective CPU billing cleos enhancement and adding subjective_cpu_bill to /v1/chain/get_account result #10250

Merged
merged 8 commits into from
Apr 17, 2021
Merged

Rel 2.0.x: Subjective CPU billing cleos enhancement and adding subjective_cpu_bill to /v1/chain/get_account result #10250

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
628e720
Add subjective_cpu_bill to get_account_results
linhuang-blockone Apr 14, 2021
20608dc
Add cleos support for cpu subjective billing
softprofe Apr 15, 2021
a61dd83
Export get_subjective_bill from producer_plugin and populate subjecti…
linhuang-blockone Apr 15, 2021
878e52a
Make subjective cpu billing a separate entity in cleos output so that…
linhuang-blockone Apr 15, 2021
25b0b37
Make subject_cpu_bill optional so all versions of cleos can work
linhuang-blockone Apr 15, 2021
4cc445a
Find producer_plugin at startup and display subjective cpu bandwidth …
linhuang-blockone Apr 15, 2021
810a28d
Use account_resource_limit to store subjective CPU billing limit and …
linhuang-blockone Apr 16, 2021
faaadc5
Fix a missing compile error, sorry
linhuang-blockone Apr 16, 2021
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
4 changes: 2 additions & 2 deletions plugins/chain_plugin/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ add_library( chain_plugin
chain_plugin.cpp
${HEADERS} )

target_link_libraries( chain_plugin eosio_chain appbase )
target_include_directories( chain_plugin PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/include" "${CMAKE_CURRENT_SOURCE_DIR}/../chain_interface/include" "${CMAKE_CURRENT_SOURCE_DIR}/../../libraries/appbase/include")
target_link_libraries( chain_plugin producer_plugin eosio_chain appbase )
Copy link
Contributor

Choose a reason for hiding this comment

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

I had no idea that would work:
https://cmake.org/cmake/help/latest/command/target_link_libraries.html#cyclic-dependencies-of-static-libraries

However, I'm not sure if we should do this or use the method providers of plugin_interface. I'm personally not a huge fan of the plugin_interface but that is the current way this is done. Thoughts @larryk85

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 will look into how to use plugin_interface.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you know an existing code example? I saw register and subscribe of method providers. But it is immediately clear to me how to use it.

target_include_directories( chain_plugin PUBLIC "${CMAKE_CURRENT_SOURCE_DIR}/include" "${CMAKE_CURRENT_SOURCE_DIR}/../chain_interface/include" "${CMAKE_CURRENT_SOURCE_DIR}/../../libraries/appbase/include" "${CMAKE_CURRENT_SOURCE_DIR}/../producer_plugin/include")

add_subdirectory( test )
6 changes: 6 additions & 0 deletions plugins/chain_plugin/chain_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
#include <eosio/chain_plugin/chain_plugin.hpp>
#include <eosio/producer_plugin/producer_plugin.hpp>
#include <eosio/chain/fork_database.hpp>
#include <eosio/chain/block_log.hpp>
#include <eosio/chain/exceptions.hpp>
Expand Down Expand Up @@ -2436,6 +2437,11 @@ read_only::get_account_results read_only::get_account( const get_account_params&
result.cpu_limit = rm.get_account_cpu_limit_ex( result.account_name, greylist_limit).first;
result.ram_usage = rm.get_account_ram_usage( result.account_name );

producer_plugin* producer_plug = app().find_plugin<producer_plugin>();
heifner marked this conversation as resolved.
Show resolved Hide resolved
if ( producer_plug ) {
result.subjective_cpu_bill = producer_plug->get_subjective_bill( result.account_name, fc::time_point::now() );
}

const auto& permissions = d.get_index<permission_index,by_owner>();
auto perm = permissions.lower_bound( boost::make_tuple( params.account_name ) );
while( perm != permissions.end() && perm->owner == params.account_name ) {
Expand Down
4 changes: 3 additions & 1 deletion plugins/chain_plugin/include/eosio/chain_plugin/chain_plugin.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ class read_only {
fc::variant refund_request;
fc::variant voter_info;
fc::variant rex_info;

optional<uint32_t> subjective_cpu_bill;
};

struct get_account_params {
Expand Down Expand Up @@ -798,7 +800,7 @@ FC_REFLECT( eosio::chain_apis::read_only::get_scheduled_transactions_result, (tr
FC_REFLECT( eosio::chain_apis::read_only::get_account_results,
(account_name)(head_block_num)(head_block_time)(privileged)(last_code_update)(created)
(core_liquid_balance)(ram_quota)(net_weight)(cpu_weight)(net_limit)(cpu_limit)(ram_usage)(permissions)
(total_resources)(self_delegated_bandwidth)(refund_request)(voter_info)(rex_info) )
(total_resources)(self_delegated_bandwidth)(refund_request)(voter_info)(rex_info)(subjective_cpu_bill) )
// @swap code_hash
FC_REFLECT( eosio::chain_apis::read_only::get_code_results, (account_name)(code_hash)(wast)(wasm)(abi) )
FC_REFLECT( eosio::chain_apis::read_only::get_code_hash_results, (account_name)(code_hash) )
Expand Down
1 change: 1 addition & 0 deletions plugins/producer_plugin/include/eosio/producer_plugin/producer_plugin.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ class producer_plugin : public appbase::plugin<producer_plugin> {

bool is_producer_key(const chain::public_key_type& key) const;
chain::signature_type sign_compact(const chain::public_key_type& key, const fc::sha256& digest) const;
uint32_t get_subjective_bill( const account_name& first_auth, const fc::time_point& now ) const;

virtual void plugin_initialize(const boost::program_options::variables_map& options);
virtual void plugin_startup();
Expand Down
5 changes: 5 additions & 0 deletions plugins/producer_plugin/producer_plugin.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -731,6 +731,11 @@ bool producer_plugin::is_producer_key(const chain::public_key_type& key) const
return false;
}

uint32_t producer_plugin::get_subjective_bill( const account_name& first_auth, const fc::time_point& now ) const
{
return my->_subjective_billing.get_subjective_bill( first_auth, now );
}

chain::signature_type producer_plugin::sign_compact(const chain::public_key_type& key, const fc::sha256& digest) const
{
if(key != chain::public_key_type()) {
Expand Down
9 changes: 8 additions & 1 deletion programs/cleos/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2304,13 +2304,20 @@ void get_account( const string& accountName, const string& coresym, bool json_fo
}
}


std::cout << std::fixed << setprecision(3);
std::cout << indent << std::left << std::setw(11) << "used:" << std::right << std::setw(18) << to_pretty_time( res.cpu_limit.used ) << "\n";
std::cout << indent << std::left << std::setw(11) << "available:" << std::right << std::setw(18) << to_pretty_time( res.cpu_limit.available ) << "\n";
std::cout << indent << std::left << std::setw(11) << "limit:" << std::right << std::setw(18) << to_pretty_time( res.cpu_limit.max ) << "\n";
std::cout << std::endl;

std::cout << "subjective cpu bandwidth:" << std::endl;
uint32_t subjective_cpu_bill = 0;
if( res.subjective_cpu_bill.valid() ) {
subjective_cpu_bill = *res.subjective_cpu_bill;
}
std::cout << indent << std::left << std::setw(11) << "used:" << std::right << std::setw(18) << to_pretty_time( subjective_cpu_bill ) << "\n";
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 it would be better to not print the section at all if not available. Most of the time it would be true to report 0 as it would not have subjective billing. However, we have released versions that have subjective billing but will not return a subjective bill amount.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will change.

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 will not log only when it is not 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.

get_subjective_bill in subjective_billing.hpp always returns a value (returning 0 when subjective billing is disabled or not available). Looks like we need another API to tell if subject billing amount is present. Do that in 2.1.x?

Copy link
Contributor

@heifner heifner Apr 16, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

There is a difference from a nodeos not having the code of this PR vs having subject billing disabled. If nodeos has this PR code then 0 is the correct response to print as that indicates either there is no subjective billing on the node or the account has no subjective billing applied to it. Either way there is no subjective billing for that account.

As for an indication if a node has subjective billing enabled or not at all, that should be a different api call. That api call if added would need to indicate if it is enable for API calls or P2P calls or both. In the future that api call might also provide information on any configurable values of subjective billing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for explanation.

std::cout << std::endl;

if( res.refund_request.is_object() ) {
auto obj = res.refund_request.get_object();
auto request_time = fc::time_point_sec::from_iso_string( obj["request_time"].as_string() );
Expand Down