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

Deep Mind off-chain ABI serializer & FC encoded hex output #9073

Merged
merged 7 commits into from
Dec 3, 2020
Merged

Deep Mind off-chain ABI serializer & FC encoded hex output #9073

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
5195064
On boot, `nodeos` will now dump all known ABIs of each account.
abourget Apr 28, 2020
734f60b
Offchain ABI serializer & fc encoded hex output
Apr 22, 2020
99813e7
Fixed compilation error when using `db.get_index<account_index>`
May 12, 2020
cdd36ed
Merge branch 'develop' into feature/offchain-abi-and-hex-output
Nov 24, 2020
3a6a8a8
Updated patch to latest deep mind version
Nov 24, 2020
adc58da
Reverted style change applied by mistake
Nov 25, 2020
786832b
Fixed PUSH_CREATE not returning fully signed transaction
Dec 1, 2020
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
22 changes: 7 additions & 15 deletions libraries/chain/apply_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -582,10 +582,6 @@ void apply_context::schedule_deferred_transaction( const uint128_t& sender_id, a
}

if (auto dm_logger = control.get_deep_mind_logger()) {
fc::datastream<const char*> ds( ptr->packed_trx.data(), ptr->packed_trx.size() );
transaction dtrx;
fc::raw::unpack(ds, dtrx);

fc_dlog(*dm_logger, "DTRX_OP MODIFY_CANCEL ${action_id} ${sender} ${sender_id} ${payer} ${published} ${delay} ${expiration} ${trx_id} ${trx}",
("action_id", get_action_id())
("sender", receiver)
Expand All @@ -594,8 +590,8 @@ void apply_context::schedule_deferred_transaction( const uint128_t& sender_id, a
("published", ptr->published)
("delay", ptr->delay_until)
("expiration", ptr->expiration)
("trx_id", dtrx.id())
("trx", control.maybe_to_variant_with_abi(dtrx, abi_serializer::create_yield_function(control.get_abi_serializer_max_time())))
("trx_id", ptr->trx_id)
("trx", fc::to_hex(ptr->packed_trx.data(), ptr->packed_trx.size()))
);
}

Expand Down Expand Up @@ -626,7 +622,7 @@ void apply_context::schedule_deferred_transaction( const uint128_t& sender_id, a
("delay", gtx.delay_until)
("expiration", gtx.expiration)
("trx_id", trx.id())
("trx", control.maybe_to_variant_with_abi(trx, abi_serializer::create_yield_function(control.get_abi_serializer_max_time())))
("trx", fc::to_hex(gtx.packed_trx.data(), gtx.packed_trx.size()))
);
}
} );
Expand Down Expand Up @@ -654,8 +650,8 @@ void apply_context::schedule_deferred_transaction( const uint128_t& sender_id, a
("published", gtx.published)
("delay", gtx.delay_until)
("expiration", gtx.expiration)
("trx_id", trx.id())
("trx", control.maybe_to_variant_with_abi(trx, abi_serializer::create_yield_function(control.get_abi_serializer_max_time())))
("trx_id", gtx.trx_id)
("trx", fc::to_hex(gtx.packed_trx.data(), gtx.packed_trx.size()))
);
}
} );
Expand All @@ -678,10 +674,6 @@ bool apply_context::cancel_deferred_transaction( const uint128_t& sender_id, acc
if ( gto ) {
std::string event_id;
if (auto dm_logger = control.get_deep_mind_logger()) {
fc::datastream<const char*> ds( gto->packed_trx.data(), gto->packed_trx.size() );
transaction dtrx;
fc::raw::unpack(ds, dtrx);

event_id = STORAGE_EVENT_ID("${id}", ("id", gto->id));

fc_dlog(*dm_logger, "DTRX_OP CANCEL ${action_id} ${sender} ${sender_id} ${payer} ${published} ${delay} ${expiration} ${trx_id} ${trx}",
Expand All @@ -692,8 +684,8 @@ bool apply_context::cancel_deferred_transaction( const uint128_t& sender_id, acc
("published", gto->published)
("delay", gto->delay_until)
("expiration", gto->expiration)
("trx_id", dtrx.id())
("trx", control.maybe_to_variant_with_abi(dtrx, abi_serializer::create_yield_function(control.get_abi_serializer_max_time())))
("trx_id", gto->trx_id)
("trx", fc::to_hex(gto->packed_trx.data(), gto->packed_trx.size()))
);
}

Expand Down
12 changes: 8 additions & 4 deletions libraries/chain/authorization_manager.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,8 +159,9 @@ namespace eosio { namespace chain {
p.auth = auth;

if (auto dm_logger = _control.get_deep_mind_logger()) {
fc_dlog(*dm_logger, "PERM_OP INS ${action_id} ${data}",
fc_dlog(*dm_logger, "PERM_OP INS ${action_id} ${permission_id} ${data}",
("action_id", action_id)
("permission_id", p.id)
("data", p)
);
}
Expand Down Expand Up @@ -198,8 +199,9 @@ namespace eosio { namespace chain {
p.auth = std::move(auth);

if (auto dm_logger = _control.get_deep_mind_logger()) {
fc_dlog(*dm_logger, "PERM_OP INS ${action_id} ${data}",
fc_dlog(*dm_logger, "PERM_OP INS ${action_id} ${permission_id} ${data}",
("action_id", action_id)
("permission_id", p.id)
("data", p)
);
}
Expand All @@ -224,8 +226,9 @@ namespace eosio { namespace chain {
po.last_updated = _control.pending_block_time();

if (auto dm_logger = _control.get_deep_mind_logger()) {
fc_dlog(*dm_logger, "PERM_OP UPD ${action_id} ${data}",
fc_dlog(*dm_logger, "PERM_OP UPD ${action_id} ${permission_id} ${data}",
("action_id", action_id)
("permission_id", po.id)
("data", fc::mutable_variant_object()
("old", old_permission)
("new", po)
Expand All @@ -244,8 +247,9 @@ namespace eosio { namespace chain {
_db.get_mutable_index<permission_usage_index>().remove_object( permission.usage_id._id );

if (auto dm_logger = _control.get_deep_mind_logger()) {
fc_dlog(*dm_logger, "PERM_OP REM ${action_id} ${data}",
fc_dlog(*dm_logger, "PERM_OP REM ${action_id} ${permission_id} ${data}",
("action_id", action_id)
("permission_id", permission.id)
("data", permission)
);
}
Expand Down
28 changes: 26 additions & 2 deletions libraries/chain/controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -700,6 +700,26 @@ struct controller_impl {
// else no checks needed since fork_db will be completely reset on replay anyway
}

if (auto dm_logger = get_deep_mind_logger()) {
// FIXME: We should probably feed that from CMake directly somehow ...
fc_dlog(*dm_logger, "DEEP_MIND_VERSION 13 0");

fc_dlog(*dm_logger, "ABIDUMP START ${block_num} ${global_sequence_num}",
("block_num", head->block_num)
("global_sequence_num", db.get<dynamic_global_property_object>().global_action_sequence)
);
const auto& idx = db.get_index<account_index>();
for (auto& row : idx.indices()) {
if (row.abi.size() != 0) {
fc_dlog(*dm_logger, "ABIDUMP ABI ${contract} ${abi}",
("contract", row.name)
("abi", row.abi)
);
}
}
fc_dlog(*dm_logger, "ABIDUMP END");
}

if( last_block_num > head->block_num ) {
replay( check_shutdown ); // replay any irreversible and reversible blocks ahead of current head
}
Expand Down Expand Up @@ -894,9 +914,11 @@ struct controller_impl {
}

if (auto dm_logger = get_deep_mind_logger()) {
auto packed_trx = fc::raw::pack(etrx);

fc_dlog(*dm_logger, "TRX_OP CREATE onerror ${id} ${trx}",
("id", etrx.id())
("trx", self.maybe_to_variant_with_abi(etrx, abi_serializer::create_yield_function(self.get_abi_serializer_max_time())))
("trx", fc::to_hex(packed_trx))
);
}

Expand Down Expand Up @@ -2253,9 +2275,11 @@ struct controller_impl {
}

if (auto dm_logger = get_deep_mind_logger()) {
auto packed_trx = fc::raw::pack(trx);

fc_dlog(*dm_logger, "TRX_OP CREATE onblock ${id} ${trx}",
("id", trx.id())
("trx", self.maybe_to_variant_with_abi(trx, abi_serializer::create_yield_function(self.get_abi_serializer_max_time())))
("trx", fc::to_hex(packed_trx))
);
}

Expand Down
5 changes: 3 additions & 2 deletions libraries/chain/transaction_context.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,7 @@ namespace eosio { namespace chain {
if (auto dm_logger = control.get_deep_mind_logger()) {
event_id = STORAGE_EVENT_ID("${id}", ("id", gto.id));

auto packed_signed_trx = fc::raw::pack(trx);
Copy link
Contributor

Choose a reason for hiding this comment

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

packed_signed_trx materially different from packed_trx ? This looks like an unnecessary "round trip" with line 626 above

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'll double check, I remember some had subtle differences when packing not giving the exact same output format, specially related to signature. One was giving an output with signature the other not, but I don't remember if this one was problematic or not.

Copy link
Contributor

Choose a reason for hiding this comment

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

So, it appears packed_trx would

  • potentially have signatures (relevant if it weren't a deferred transaction)
  • be potentially compressed (relevant if it weren't a deferred transaction)
  • represent the (un)?compressed transaction as a blob of data and contain some envelope information about the blob's format

by re-packing it it should always be uncompressed and only the transaction BUT i don't think it will have signatures.

packed_transaction::get_transaction() const returns a transaction not a signed_transaction. Also, thisis a deferred transaction which has no signatures. Please rename the variable to something like serialized_trx so that future readers do not expect this blob to contain signatures.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In version 2.x of deep mind branch, this was outputting signature (for delayed pushed transaction which is the case here). You are right that is not a signed_trx anymore here, it's a plain transaction without the signature.

Now, I checked and using fc::to_hex(packed_trx.get_packed_transaction().data(), packed_trx.get_packed_transaction().size()) does not print signature neither. That would have been a correct change since we could have uncompress the transaction if it was compressed.

Now, I think my best way of handling it here is to also output the signatures directly, hoping they are not pruned which seems the case here.

This does not compile yet because error: no member named 'visit' in 'fc::reflector<const std::__1::vector<fc::crypto::signature, std::__1::allocator<fc::crypto::signature> > *>' when used in a mutable variable object for printing like this:

            fc_dlog(*dm_logger, "DTRX_OP PUSH_CREATE ${action_id} ${sender} ${sender_id} ${payer} ${published} ${delay} ${expiration} ${trx_id} ${trx} ${signatures}",
               ("action_id", get_action_id())
...
               ("signatures", packed_trx.get_signatures())
            );

I still have some holes in my understanding of the bigger serialization format around fc and EOSIO codebase, but I don't see yet why it complains like this. The fc::crypto::signature has specialized to_variant and from_variant overload as well as having a dedicated FC_REFLECT(fc::crypto::signature, (_storage) ) definition.

So maybe you could give me some hint here to find why it's not working.

Copy link
Contributor

Choose a reason for hiding this comment

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

You have two options here:

  1. maintain the course with your new serialization, you should dereference the pointer so that the mutable_variant_object receives a const std::vector<signature_type>& instead of a pointer.

  2. go back to the original and change it to:

auto packed_signed_trx = fc::raw::pack(packed_trx.to_packed_transaction_v0().get_signed_transaction());

which should give you what you were seeing in the 2.x version

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 the tips.

I went with the original output and the extra hop of turning it into a v0 packed transaction. It was easier to go that route, specially since I would also needed to add context_free_data.

fc_dlog(*dm_logger, "DTRX_OP PUSH_CREATE ${action_id} ${sender} ${sender_id} ${payer} ${published} ${delay} ${expiration} ${trx_id} ${trx}",
("action_id", get_action_id())
("sender", gto.sender)
Expand All @@ -655,8 +656,8 @@ namespace eosio { namespace chain {
("published", gto.published)
("delay", gto.delay_until)
("expiration", gto.expiration)
("trx_id", trx.id())
("trx", control.maybe_to_variant_with_abi(trx, abi_serializer::create_yield_function(control.get_abi_serializer_max_time())))
("trx_id", gto.trx_id)
("trx", fc::to_hex(packed_signed_trx.data(), packed_signed_trx.size()))
);
}
});
Expand Down
Loading