From f78cbaa22d6f925cb3c73593fe60785b4c0f63c3 Mon Sep 17 00:00:00 2001 From: Nathaniel Date: Sun, 26 Feb 2023 12:09:49 -0600 Subject: [PATCH 1/3] Ref #88: Implement wrapper for get_code_hash --- libraries/eosiolib/capi/eosio/action.h | 10 +++++ libraries/eosiolib/contracts/eosio/action.hpp | 39 +++++++++++++++++++ tests/unit/test_contracts/capi/action.c | 1 + 3 files changed, 50 insertions(+) diff --git a/libraries/eosiolib/capi/eosio/action.h b/libraries/eosiolib/capi/eosio/action.h index 58d345848a..6f51526384 100644 --- a/libraries/eosiolib/capi/eosio/action.h +++ b/libraries/eosiolib/capi/eosio/action.h @@ -168,6 +168,16 @@ uint64_t publication_time( void ); __attribute__((eosio_wasm_import)) capi_name current_receiver( void ); +/** + * Get the hash of the code currently published on the given account + * @param account Name of the account to hash the code of + * @param struct_version Version specifying the desired format of the returned struct + * @param result_buffer Buffer wherein the result should be written + * @param buffer_size Size in bytes of result_buffer + */ + __attribute__((eosio_wasm_import)) + uint32_t get_code_hash( uint64_t account, uint32_t struct_version, char* result_buffer, size_t buffer_size ); + /** * Set the action return value which will be included in the action_receipt * @brief Set the action return value diff --git a/libraries/eosiolib/contracts/eosio/action.hpp b/libraries/eosiolib/contracts/eosio/action.hpp index 2ec2163013..ba56394ced 100644 --- a/libraries/eosiolib/contracts/eosio/action.hpp +++ b/libraries/eosiolib/contracts/eosio/action.hpp @@ -8,6 +8,7 @@ #include "../../core/eosio/serialize.hpp" #include "../../core/eosio/datastream.hpp" #include "../../core/eosio/name.hpp" +#include "../../core/eosio/fixed_bytes.hpp" #include "../../core/eosio/ignore.hpp" #include "../../core/eosio/time.hpp" @@ -52,9 +53,23 @@ namespace eosio { __attribute__((eosio_wasm_import)) uint64_t current_receiver(); + + __attribute__((eosio_wasm_import)) + uint32_t get_code_hash( uint64_t account, uint32_t struct_version, char* result_buffer, size_t buffer_size ); } }; + struct code_hash_result { + unsigned_int struct_version; + uint64_t code_sequence; + checksum256 code_hash; + uint8_t vm_type; + uint8_t vm_version; + + CDT_REFLECT(struct_version, code_sequence, code_hash, vm_type, vm_version); + EOSLIB_SERIALIZE(code_hash_result, (struct_version)(code_sequence)(code_hash)(vm_type)(vm_version)); + }; + /** * @defgroup action Action * @ingroup contracts @@ -150,6 +165,30 @@ namespace eosio { return name{internal_use_do_not_use::current_receiver()}; } + /** + * Get the hash of the code currently published on the given account + * @param account Name of the account to hash the code of + * @param full_result Optional: If a full result struct is desired, a pointer to the struct to populate + * @return The SHA256 hash of the specified account's code + */ + inline checksum256 get_code_hash( name account, code_hash_result* full_result = nullptr ) { + if (full_result == nullptr) + full_result = (code_hash_result*)alloca(sizeof(code_hash_result)); + + // Packed size is dynamic, so we don't know what it'll be. Try to have plenty of space. + auto struct_buffer_size = sizeof(code_hash_result)*2; + char* struct_buffer = (char*)alloca(struct_buffer_size); + + using VersionType = decltype(code_hash_result::struct_version); + const VersionType STRUCT_VERSION = 0; + internal_use_do_not_use::get_code_hash(account.value, STRUCT_VERSION, struct_buffer, struct_buffer_size); + check(unpack(struct_buffer, struct_buffer_size) == STRUCT_VERSION, + "Hypervisor returned unexpected code hash struct version"); + unpack(*full_result, struct_buffer, struct_buffer_size); + + return full_result->code_hash; + } + /** * Copy up to length bytes of current action data to the specified location * diff --git a/tests/unit/test_contracts/capi/action.c b/tests/unit/test_contracts/capi/action.c index 63824f2bff..a489299f87 100644 --- a/tests/unit/test_contracts/capi/action.c +++ b/tests/unit/test_contracts/capi/action.c @@ -13,5 +13,6 @@ void test_action( void ) { send_context_free_inline(NULL, 0); publication_time(); current_receiver(); + get_code_hash(0, 0, NULL, 0); set_action_return_value(NULL, 0); } From f7b206fe62f8c213a71c573f72af2d276d268bfd Mon Sep 17 00:00:00 2001 From: Nathaniel Date: Sun, 26 Feb 2023 13:33:34 -0600 Subject: [PATCH 2/3] Resolve #88: Testing for get_code_hash --- tests/integration/contracts.hpp.in | 5 ++ tests/integration/get_code_hash_tests.cpp | 46 +++++++++++++++++++ tests/unit/test_contracts/CMakeLists.txt | 2 + .../test_contracts/get_code_hash_read.cpp | 27 +++++++++++ .../test_contracts/get_code_hash_table.hpp | 10 ++++ .../test_contracts/get_code_hash_write.cpp | 28 +++++++++++ 6 files changed, 118 insertions(+) create mode 100644 tests/integration/get_code_hash_tests.cpp create mode 100644 tests/unit/test_contracts/get_code_hash_read.cpp create mode 100644 tests/unit/test_contracts/get_code_hash_table.hpp create mode 100644 tests/unit/test_contracts/get_code_hash_write.cpp diff --git a/tests/integration/contracts.hpp.in b/tests/integration/contracts.hpp.in index abbb050c78..d9c40c74e1 100644 --- a/tests/integration/contracts.hpp.in +++ b/tests/integration/contracts.hpp.in @@ -22,5 +22,10 @@ namespace eosio::testing { static std::vector crypto_primitives_test_wasm() { return read_wasm("${CMAKE_BINARY_DIR}/../unit/test_contracts/crypto_primitives_tests.wasm"); } static std::vector crypto_primitives_test_abi() { return read_abi("${CMAKE_BINARY_DIR}/../unit/test_contracts/crypto_primitives_tests.abi"); } + + static std::vector get_code_hash_write_test_wasm() { return read_wasm("${CMAKE_BINARY_DIR}/../unit/test_contracts/get_code_hash_write.wasm"); } + static std::vector get_code_hash_write_test_abi() { return read_abi("${CMAKE_BINARY_DIR}/../unit/test_contracts/get_code_hash_write.abi"); } + static std::vector get_code_hash_read_test_wasm() { return read_wasm("${CMAKE_BINARY_DIR}/../unit/test_contracts/get_code_hash_read.wasm"); } + static std::vector get_code_hash_read_test_abi() { return read_abi("${CMAKE_BINARY_DIR}/../unit/test_contracts/get_code_hash_read.abi"); } }; } //ns eosio::testing diff --git a/tests/integration/get_code_hash_tests.cpp b/tests/integration/get_code_hash_tests.cpp new file mode 100644 index 0000000000..cdce928176 --- /dev/null +++ b/tests/integration/get_code_hash_tests.cpp @@ -0,0 +1,46 @@ +#include +#include +#include + +#include + +#include + +#include + +using namespace eosio; +using namespace eosio::testing; +using namespace eosio::chain; +using namespace fc; + +using mvo = fc::mutable_variant_object; + +struct code_hash { + uint64_t id; + fc::sha256 hash; + uint64_t primary_key() const { return id; } +}; +FC_REFLECT(code_hash, (id)(hash)) + +BOOST_AUTO_TEST_SUITE(get_code_hash_tests_suite) + +BOOST_FIXTURE_TEST_CASE( get_code_hash_tests, tester ) try { + create_accounts( { "test"_n } ); + produce_block(); + + set_code( "test"_n, contracts::get_code_hash_write_test_wasm() ); + set_abi( "test"_n, contracts::get_code_hash_write_test_abi().data() ); + + produce_blocks(); + push_action("test"_n, "theaction"_n, "test"_n, mvo()); + code_hash entry; + get_table_entry(entry, "test"_n, "test"_n, "code.hash"_n, 0); + wdump((entry.hash)); + + set_code( "test"_n, contracts::get_code_hash_read_test_wasm() ); + produce_blocks(); + + push_action("test"_n, "theaction"_n, "test"_n, mvo()); +} FC_LOG_AND_RETHROW() + +BOOST_AUTO_TEST_SUITE_END() diff --git a/tests/unit/test_contracts/CMakeLists.txt b/tests/unit/test_contracts/CMakeLists.txt index 1e418f38b0..3b1654fe9f 100644 --- a/tests/unit/test_contracts/CMakeLists.txt +++ b/tests/unit/test_contracts/CMakeLists.txt @@ -7,6 +7,8 @@ add_contract(explicit_nested_tests explicit_nested_tests explicit_nested_tests.c add_contract(transfer_contract transfer_contract transfer.cpp) add_contract(minimal_tests minimal_tests minimal_tests.cpp) add_contract(crypto_primitives_tests crypto_primitives_tests crypto_primitives_tests.cpp) +add_contract(get_code_hash_tests get_code_hash_write get_code_hash_write.cpp) +add_contract(get_code_hash_tests get_code_hash_read get_code_hash_read.cpp) add_contract(capi_tests capi_tests capi/capi.c capi/action.c capi/chain.c capi/crypto.c capi/db.c capi/permission.c capi/print.c capi/privileged.c capi/system.c capi/transaction.c) diff --git a/tests/unit/test_contracts/get_code_hash_read.cpp b/tests/unit/test_contracts/get_code_hash_read.cpp new file mode 100644 index 0000000000..05c51a70c4 --- /dev/null +++ b/tests/unit/test_contracts/get_code_hash_read.cpp @@ -0,0 +1,27 @@ +#include +#include +#include + +#include "get_code_hash_table.hpp" + +class [[eosio::contract]] get_code_hash_tests : public contract { +public: + using contract::contract; + + using hash_table = multi_index; + + // Read the old code's hash from database and verify new code's hash differs + [[eosio::action]] + void theaction() { + require_auth(get_self()); + hash_table hashes(get_self(), get_self().value); + + auto hash = get_code_hash(get_self()); + check(hash != checksum256(), "Code hash should not be null"); + + auto record = hashes.get(0, "Unable to find recorded hash"); + check(hash != record.hash, "Code hash has not changed"); + eosio::print("Old hash: ", record.hash, "; new hash: ", hash); + } +}; + diff --git a/tests/unit/test_contracts/get_code_hash_table.hpp b/tests/unit/test_contracts/get_code_hash_table.hpp new file mode 100644 index 0000000000..696558e40a --- /dev/null +++ b/tests/unit/test_contracts/get_code_hash_table.hpp @@ -0,0 +1,10 @@ +#pragma once + +using namespace eosio; + +TABLE code_hash { + uint64_t id; + checksum256 hash; + uint64_t primary_key() const { return id; } +}; + diff --git a/tests/unit/test_contracts/get_code_hash_write.cpp b/tests/unit/test_contracts/get_code_hash_write.cpp new file mode 100644 index 0000000000..ae61a06a07 --- /dev/null +++ b/tests/unit/test_contracts/get_code_hash_write.cpp @@ -0,0 +1,28 @@ +#include +#include +#include + +#include "get_code_hash_table.hpp" + +class [[eosio::contract]] get_code_hash_tests : public contract { +public: + using contract::contract; + + using hash_table = multi_index; + + // Write this code's hash to database + [[eosio::action]] + void theaction() { + require_auth(get_self()); + hash_table hashes(get_self(), get_self().value); + + auto hash = get_code_hash(get_self()); + check(hash != checksum256(), "Code hash should not be null"); + + hashes.emplace(get_self(), [&hash](auto& t) { + t.id = 0; + t.hash = hash; + }); + } +}; + From 887b129bde0a40d6c566f29b7f63fcf79413e1d4 Mon Sep 17 00:00:00 2001 From: Nathaniel Date: Mon, 13 Mar 2023 21:26:55 -0500 Subject: [PATCH 3/3] Ref #88 Review updates: check retval of intrinsic Ensure the buffer was adequately large for the response and if not, allocate sufficient space and call intrinsic again. --- libraries/eosiolib/contracts/eosio/action.hpp | 26 ++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/libraries/eosiolib/contracts/eosio/action.hpp b/libraries/eosiolib/contracts/eosio/action.hpp index ba56394ced..69ee153359 100644 --- a/libraries/eosiolib/contracts/eosio/action.hpp +++ b/libraries/eosiolib/contracts/eosio/action.hpp @@ -174,18 +174,38 @@ namespace eosio { inline checksum256 get_code_hash( name account, code_hash_result* full_result = nullptr ) { if (full_result == nullptr) full_result = (code_hash_result*)alloca(sizeof(code_hash_result)); + constexpr size_t max_stack_buffer_size = 50; - // Packed size is dynamic, so we don't know what it'll be. Try to have plenty of space. - auto struct_buffer_size = sizeof(code_hash_result)*2; + // Packed size of this struct will virtually always be less than the struct size; always less after padding + auto struct_buffer_size = sizeof(code_hash_result); char* struct_buffer = (char*)alloca(struct_buffer_size); using VersionType = decltype(code_hash_result::struct_version); const VersionType STRUCT_VERSION = 0; - internal_use_do_not_use::get_code_hash(account.value, STRUCT_VERSION, struct_buffer, struct_buffer_size); + auto response_size = + internal_use_do_not_use::get_code_hash(account.value, STRUCT_VERSION, struct_buffer, struct_buffer_size); + // Safety check: in this case, response size should never exceed our buffer, but just in case... + bool buffer_on_heap = false; + if (response_size > struct_buffer_size) { + // Slow path: allocate an adequate buffer and try again + // No need to deallocate struct_buffer since it was alloca'd + if (response_size > max_stack_buffer_size) { + struct_buffer = (char*)malloc(response_size); + buffer_on_heap = true; + } else { + struct_buffer = (char*)alloca(response_size); + } + internal_use_do_not_use::get_code_hash(account.value, STRUCT_VERSION, struct_buffer, struct_buffer_size); + } + check(unpack(struct_buffer, struct_buffer_size) == STRUCT_VERSION, "Hypervisor returned unexpected code hash struct version"); unpack(*full_result, struct_buffer, struct_buffer_size); + // If struct_buffer is heap allocated, we must free it + if (buffer_on_heap) + free(struct_buffer); + return full_result->code_hash; }