Refactor packed_transaction for better encapsulation

libraries/chain/include/eosio/chain/abi_serializer.hpp

@@ -418,11 +418,11 @@ namespace impl {
          mutable_variant_object mvo;
          auto trx = ptrx.get_transaction();
          mvo("id", trx.id());
-         mvo("signatures", ptrx.signatures);
-         mvo("compression", ptrx.compression);
-         mvo("packed_context_free_data", ptrx.packed_context_free_data);
+         mvo("signatures", ptrx.get_signatures());
+         mvo("compression", ptrx.get_compression());
+         mvo("packed_context_free_data", ptrx.get_packed_context_free_data());
          mvo("context_free_data", ptrx.get_context_free_data());
-         mvo("packed_trx", ptrx.packed_trx);
+         mvo("packed_trx", ptrx.get_packed_transaction());
          add(mvo, "transaction", trx, resolver, ctx);
 
          out(name, std::move(mvo));
@@ -577,32 +577,33 @@ namespace impl {
          const variant_object& vo = v.get_object();
          EOS_ASSERT(vo.contains("signatures"), packed_transaction_type_exception, "Missing signatures");
          EOS_ASSERT(vo.contains("compression"), packed_transaction_type_exception, "Missing compression");
-         from_variant(vo["signatures"], ptrx.signatures);
-         from_variant(vo["compression"], ptrx.compression);
+         std::vector<signature_type> signatures;
+         packed_transaction::compression_type compression;
+         from_variant(vo["signatures"], signatures);
+         from_variant(vo["compression"], compression);
 
-         // TODO: Make this nicer eventually. But for now, if it works... good enough.
+         bytes packed_cfd;
          if( vo.contains("packed_trx") && vo["packed_trx"].is_string() && !vo["packed_trx"].as_string().empty() ) {
-            from_variant(vo["packed_trx"], ptrx.packed_trx);
-            auto trx = ptrx.get_transaction(); // Validates transaction data provided.
+            bytes packed_trx;
+            std::vector<bytes> cfd;
+            from_variant(vo["packed_trx"], packed_trx);
             if( vo.contains("packed_context_free_data") && vo["packed_context_free_data"].is_string() && !vo["packed_context_free_data"].as_string().empty() ) {
-               from_variant(vo["packed_context_free_data"], ptrx.packed_context_free_data );
+               from_variant(vo["packed_context_free_data"], packed_cfd );
             } else if( vo.contains("context_free_data") ) {
-               vector<bytes> context_free_data;
-               from_variant(vo["context_free_data"], context_free_data);
-               ptrx.set_transaction(trx, context_free_data, ptrx.compression);
+               from_variant(vo["context_free_data"], cfd);
             }
+            ptrx = packed_transaction( std::move(packed_trx), std::move(signatures), std::move(packed_cfd), std::move(cfd), compression );
          } else {
             EOS_ASSERT(vo.contains("transaction"), packed_transaction_type_exception, "Missing transaction");
-            transaction trx;
-            vector<bytes> context_free_data;
+            signed_transaction trx;
+            trx.signatures = std::move(signatures);
             extract(vo["transaction"], trx, resolver, ctx);
             if( vo.contains("packed_context_free_data") && vo["packed_context_free_data"].is_string() && !vo["packed_context_free_data"].as_string().empty() ) {
-               from_variant(vo["packed_context_free_data"], ptrx.packed_context_free_data );
-               context_free_data = ptrx.get_context_free_data();
+               from_variant(vo["packed_context_free_data"], packed_cfd );
             } else if( vo.contains("context_free_data") ) {
-               from_variant(vo["context_free_data"], context_free_data);
+               from_variant(vo["context_free_data"], trx.context_free_data );
             }
-            ptrx.set_transaction(trx, context_free_data, ptrx.compression);
+            ptrx = packed_transaction( std::move(trx), std::move(packed_cfd), compression );
          }
       }
    };

libraries/chain/include/eosio/chain/transaction.hpp

@@ -108,27 +108,29 @@ namespace eosio { namespace chain {
       packed_transaction& operator=(packed_transaction&&) = default;
 
       explicit packed_transaction(const signed_transaction& t, compression_type _compression = none)
-      :signatures(t.signatures)
+      :signatures(t.signatures), compression(_compression)
       {
-         set_transaction(t, t.context_free_data, _compression);
+         set_transaction(t);
+         set_context_free_data(t.context_free_data);
       }
 
       explicit packed_transaction(signed_transaction&& t, compression_type _compression = none)
-      :signatures(std::move(t.signatures))
+      :signatures(std::move(t.signatures)), compression(_compression)
       {
-         set_transaction(t, t.context_free_data, _compression);
+         set_transaction(t);
+         set_context_free_data(t.context_free_data);
       }
 
+      // used by abi_serializer
+      explicit packed_transaction( bytes&& packed_txn, vector<signature_type>&& sigs,
+                                   bytes&& packed_cfd, vector<bytes>&& cfd, compression_type _compression );
+      explicit packed_transaction( signed_transaction&& t, bytes&& packed_cfd, compression_type _compression );
+
       uint32_t get_unprunable_size()const;
       uint32_t get_prunable_size()const;
 
       digest_type packed_digest()const;
 
-      vector<signature_type>                  signatures;
-      fc::enum_type<uint8_t,compression_type> compression;
-      bytes                                   packed_context_free_data;
-      bytes                                   packed_trx;
-
       time_point_sec     expiration()const;
       transaction_id_type id()const;
       transaction_id_type get_uncached_id()const; // thread safe
@@ -136,12 +138,26 @@ namespace eosio { namespace chain {
       vector<bytes>      get_context_free_data()const;
       transaction        get_transaction()const;
       signed_transaction get_signed_transaction()const;
-      void               set_transaction(const transaction& t, compression_type _compression = none);
-      void               set_transaction(const transaction& t, const vector<bytes>& cfd, compression_type _compression = none);
+
+      const vector<signature_type>& get_signatures()const { return signatures; }
+      const fc::enum_type<uint8_t,compression_type>& get_compression()const { return compression; }
+      const bytes& get_packed_context_free_data()const { return packed_context_free_data; }
+      const bytes& get_packed_transaction()const { return packed_trx; }
 
    private:
-      mutable optional<transaction>           unpacked_trx; // <-- intermediate buffer used to retrieve values
       void local_unpack()const;
+      void set_transaction(const transaction& t);
+      void set_context_free_data(const vector<bytes>& cfd);
+
+      friend struct fc::reflector<packed_transaction>;
+   private:
+      vector<signature_type>                  signatures;
+      fc::enum_type<uint8_t,compression_type> compression;
+      bytes                                   packed_context_free_data;
+      bytes                                   packed_trx;
+
+   private:
+      mutable optional<transaction>           unpacked_trx; // <-- intermediate buffer used to retrieve values
    };
 
    using packed_transaction_ptr = std::shared_ptr<packed_transaction>;

libraries/chain/transaction.cpp

@@ -344,10 +344,35 @@ signed_transaction packed_transaction::get_signed_transaction() const
 
 }
 
-void packed_transaction::set_transaction(const transaction& t, packed_transaction::compression_type _compression)
+packed_transaction::packed_transaction( bytes&& packed_txn, vector<signature_type>&& sigs,
+                                        bytes&& packed_cfd, vector<bytes>&& cfd, compression_type _compression )
+:signatures(std::move(sigs))
+,compression(_compression)
+,packed_context_free_data(std::move(packed_cfd))
+,packed_trx(std::move(packed_txn))
+{
+   EOS_ASSERT(packed_cfd.empty() || cfd.empty(), tx_decompression_error, "Invalid packed_transaction");
+   if( !cfd.empty() ) {
+      set_context_free_data(cfd);
+   }
+}
+
+packed_transaction::packed_transaction( signed_transaction&& t, bytes&& packed_cfd, compression_type _compression )
+:signatures(std::move(t.signatures))
+,compression(_compression)
+,packed_context_free_data(std::move(packed_cfd))
+{
+   set_transaction(t);
+   // allow passed in packed_cfd to overwrite signed_transaction.context_free_data if provided
+   if( packed_context_free_data.empty() ) {
+      set_context_free_data(t.context_free_data);
+   }
+}
+
+void packed_transaction::set_transaction(const transaction& t)
 {
    try {
-      switch(_compression) {
+      switch(compression) {
          case none:
             packed_trx = pack_transaction(t);
             break;
@@ -357,28 +382,23 @@ void packed_transaction::set_transaction(const transaction& t, packed_transactio
          default:
             EOS_THROW(unknown_transaction_compression, "Unknown transaction compression algorithm");
       }
-   } FC_CAPTURE_AND_RETHROW((_compression)(t))
-   packed_context_free_data.clear();
-   compression = _compression;
+   } FC_CAPTURE_AND_RETHROW((compression)(t))
 }
 
-void packed_transaction::set_transaction(const transaction& t, const vector<bytes>& cfd, packed_transaction::compression_type _compression)
+void packed_transaction::set_context_free_data(const vector<bytes>& cfd)
 {
    try {
-      switch(_compression) {
+      switch(compression) {
          case none:
-            packed_trx = pack_transaction(t);
             packed_context_free_data = pack_context_free_data(cfd);
             break;
          case zlib:
-            packed_trx = zlib_compress_transaction(t);
             packed_context_free_data = zlib_compress_context_free_data(cfd);
             break;
          default:
             EOS_THROW(unknown_transaction_compression, "Unknown transaction compression algorithm");
       }
-   } FC_CAPTURE_AND_RETHROW((_compression)(t))
-   compression = _compression;
+   } FC_CAPTURE_AND_RETHROW((compression))
 }
 
 

unittests/misc_tests.cpp

@@ -590,11 +590,9 @@ BOOST_AUTO_TEST_CASE(transaction_test) { try {
    BOOST_CHECK_EQUAL(1, trx.signatures.size());
    trx.validate();
 
-   packed_transaction pkt;
-   pkt.set_transaction(trx, packed_transaction::none);
+   packed_transaction pkt(trx, packed_transaction::none);
 
-   packed_transaction pkt2;
-   pkt2.set_transaction(trx, packed_transaction::zlib);
+   packed_transaction pkt2(trx, packed_transaction::zlib);
 
    BOOST_CHECK_EQUAL(true, trx.expiration ==  pkt.expiration());
    BOOST_CHECK_EQUAL(true, trx.expiration == pkt2.expiration());