Skip to content

Commit b578043

Browse files
godexsoftgodexsoft
committed
Address review comments
1 parent c468468 commit b578043

4 files changed

Lines changed: 47 additions & 37 deletions

File tree

‎src/feed/impl/ProposedTransactionFeed.cpp‎

Lines changed: 13 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -40,19 +40,19 @@ namespace feed::impl {
4040

4141
void
4242
ProposedTransactionFeed::ProposedTransactionSlot::operator()(
43-
AllVersionMsgsType const& allVersionMsgs
43+
AllVersionsMsgsPtrType const& allVersionMsgs
4444
) const
4545
{
46-
if (auto connectionPtr = subscriptionContextWeakPtr.lock()) {
47-
if (feed.get().notified_.contains(connectionPtr.get()))
46+
if (auto connectionPtr = subscriptionContextWeakPtr_.lock()) {
47+
if (feed_.get().notified_.contains(connectionPtr.get()))
4848
return;
4949

50-
feed.get().notified_.insert(connectionPtr.get());
50+
feed_.get().notified_.insert(connectionPtr.get());
5151

5252
if (connectionPtr->apiSubversion() < 2u) {
53-
connectionPtr->send(allVersionMsgs[0]);
53+
connectionPtr->send(std::shared_ptr<std::string>(allVersionMsgs, &allVersionMsgs->v1));
5454
} else {
55-
connectionPtr->send(allVersionMsgs[1]);
55+
connectionPtr->send(std::shared_ptr<std::string>(allVersionMsgs, &allVersionMsgs->v2));
5656
}
5757
}
5858
}
@@ -107,11 +107,8 @@ ProposedTransactionFeed::unsub(
107107
void
108108
ProposedTransactionFeed::pub(boost::json::object const& receivedTxJson)
109109
{
110-
// v1: forward as-is (rippled sends "transaction" key)
111-
auto const v1Msg = std::make_shared<std::string>(boost::json::serialize(receivedTxJson));
112-
113110
// v2: rename "transaction" → "tx_json", move "hash" to top level
114-
auto const v2Msg = [&]() {
111+
auto const v2Json = [&]() {
115112
boost::json::object v2Json = receivedTxJson;
116113
if (v2Json.contains(JS(transaction))) {
117114
boost::json::value txVal = v2Json.at(JS(transaction));
@@ -125,10 +122,14 @@ ProposedTransactionFeed::pub(boost::json::object const& receivedTxJson)
125122
}
126123
v2Json[JS(tx_json)] = std::move(txVal);
127124
}
128-
return std::make_shared<std::string>(boost::json::serialize(v2Json));
125+
return v2Json;
129126
}();
130127

131-
AllVersionMsgsType const allVersionMsgs{v1Msg, v2Msg};
128+
auto const allVersionMsgs = std::make_shared<AllVersionMsgsType>(
129+
// v1: forward as-is (rippled sends "transaction" key)
130+
boost::json::serialize(receivedTxJson),
131+
boost::json::serialize(v2Json)
132+
);
132133

133134
auto const transaction = receivedTxJson.at(JS(transaction)).as_object();
134135
auto const accounts = rpc::getAccountsFromTransaction(transaction);

‎src/feed/impl/ProposedTransactionFeed.hpp‎

Lines changed: 15 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
#include <xrpl/protocol/Book.h>
3737
#include <xrpl/protocol/LedgerHeader.h>
3838

39-
#include <array>
4039
#include <cstdint>
4140
#include <functional>
4241
#include <memory>
@@ -49,23 +48,29 @@ namespace feed::impl {
4948
* @brief Feed that publishes the Proposed Transactions.
5049
*/
5150
class ProposedTransactionFeed {
52-
// Hold two versions of transaction messages: [0] = v1, [1] = v2
53-
using AllVersionMsgsType = std::array<std::shared_ptr<std::string>, 2>;
51+
// Hold two versions of transaction messages
52+
struct AllVersionMsgsType {
53+
std::string v1;
54+
std::string v2;
55+
};
56+
57+
using AllVersionsMsgsPtrType = std::shared_ptr<AllVersionMsgsType>;
5458

55-
struct ProposedTransactionSlot {
56-
std::reference_wrapper<ProposedTransactionFeed> feed;
57-
std::weak_ptr<Subscriber> subscriptionContextWeakPtr;
59+
class ProposedTransactionSlot {
60+
std::reference_wrapper<ProposedTransactionFeed> feed_;
61+
std::weak_ptr<Subscriber> subscriptionContextWeakPtr_;
5862

63+
public:
5964
ProposedTransactionSlot(
6065
ProposedTransactionFeed& feed,
6166
SubscriberSharedPtr const& connection
6267
)
63-
: feed(feed), subscriptionContextWeakPtr(connection)
68+
: feed_(feed), subscriptionContextWeakPtr_(connection)
6469
{
6570
}
6671

6772
void
68-
operator()(AllVersionMsgsType const& allVersionMsgs) const;
73+
operator()(AllVersionsMsgsPtrType const& allVersionMsgs) const;
6974
};
7075

7176
util::Logger logger_{"Subscriptions"};
@@ -76,8 +81,8 @@ class ProposedTransactionFeed {
7681
std::reference_wrapper<util::prometheus::GaugeInt> subAllCount_;
7782
std::reference_wrapper<util::prometheus::GaugeInt> subAccountCount_;
7883

79-
TrackableSignalMap<ripple::AccountID, Subscriber, AllVersionMsgsType const&> accountSignal_;
80-
TrackableSignal<Subscriber, AllVersionMsgsType const&> signal_;
84+
TrackableSignalMap<ripple::AccountID, Subscriber, AllVersionsMsgsPtrType> accountSignal_;
85+
TrackableSignal<Subscriber, AllVersionsMsgsPtrType> signal_;
8186

8287
public:
8388
/**

‎src/feed/impl/TransactionFeed.cpp‎

Lines changed: 8 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,9 @@
5656
namespace feed::impl {
5757

5858
void
59-
TransactionFeed::TransactionSlot::operator()(AllVersionTransactionsType const& allVersionMsgs) const
59+
TransactionFeed::TransactionSlot::operator()(
60+
std::shared_ptr<AllVersionsMsgsType> const& allVersionMsgs
61+
) const
6062
{
6163
if (auto connection = subscriptionContextWeakPtr.lock(); connection) {
6264
// Check if this connection already sent
@@ -66,10 +68,10 @@ TransactionFeed::TransactionSlot::operator()(AllVersionTransactionsType const& a
6668
feed.get().notified_.insert(connection.get());
6769

6870
if (connection->apiSubversion() < 2u) {
69-
connection->send(allVersionMsgs[0]);
71+
connection->send(std::shared_ptr<std::string>(allVersionMsgs, &allVersionMsgs->v1));
7072
return;
7173
}
72-
connection->send(allVersionMsgs[1]);
74+
connection->send(std::shared_ptr<std::string>(allVersionMsgs, &allVersionMsgs->v2));
7375
}
7476
}
7577

@@ -282,10 +284,9 @@ TransactionFeed::pub(
282284
return pubObj;
283285
};
284286

285-
AllVersionTransactionsType allVersionsMsgs{
286-
std::make_shared<std::string>(boost::json::serialize(genJsonByVersion(1u))),
287-
std::make_shared<std::string>(boost::json::serialize(genJsonByVersion(2u)))
288-
};
287+
auto allVersionsMsgs = std::make_shared<AllVersionsMsgsType>(
288+
boost::json::serialize(genJsonByVersion(1u)), boost::json::serialize(genJsonByVersion(2u))
289+
);
289290

290291
auto const affectedAccountsFlat = meta->getAffectedAccounts();
291292
auto affectedAccounts = std::unordered_set<ripple::AccountID>(

‎src/feed/impl/TransactionFeed.hpp‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,6 @@
3838
#include <xrpl/protocol/Book.h>
3939
#include <xrpl/protocol/LedgerHeader.h>
4040

41-
#include <array>
4241
#include <cstdint>
4342
#include <functional>
4443
#include <memory>
@@ -49,7 +48,10 @@ namespace feed::impl {
4948

5049
class TransactionFeed {
5150
// Hold two versions of transaction messages
52-
using AllVersionTransactionsType = std::array<std::shared_ptr<std::string>, 2>;
51+
struct AllVersionsMsgsType {
52+
std::string v1;
53+
std::string v2;
54+
};
5355

5456
struct TransactionSlot {
5557
std::reference_wrapper<TransactionFeed> feed;
@@ -61,7 +63,7 @@ class TransactionFeed {
6163
}
6264

6365
void
64-
operator()(AllVersionTransactionsType const& allVersionMsgs) const;
66+
operator()(std::shared_ptr<AllVersionsMsgsType> const& allVersionMsgs) const;
6567
};
6668

6769
util::Logger logger_{"Subscriptions"};
@@ -71,15 +73,16 @@ class TransactionFeed {
7173
std::reference_wrapper<util::prometheus::GaugeInt> subAccountCount_;
7274
std::reference_wrapper<util::prometheus::GaugeInt> subBookCount_;
7375

74-
TrackableSignalMap<ripple::AccountID, Subscriber, AllVersionTransactionsType const&>
76+
TrackableSignalMap<ripple::AccountID, Subscriber, std::shared_ptr<AllVersionsMsgsType> const&>
7577
accountSignal_;
76-
TrackableSignalMap<ripple::Book, Subscriber, AllVersionTransactionsType const&> bookSignal_;
77-
TrackableSignal<Subscriber, AllVersionTransactionsType const&> signal_;
78+
TrackableSignalMap<ripple::Book, Subscriber, std::shared_ptr<AllVersionsMsgsType> const&>
79+
bookSignal_;
80+
TrackableSignal<Subscriber, std::shared_ptr<AllVersionsMsgsType> const&> signal_;
7881

7982
// Signals for proposed tx subscribers
80-
TrackableSignalMap<ripple::AccountID, Subscriber, AllVersionTransactionsType const&>
83+
TrackableSignalMap<ripple::AccountID, Subscriber, std::shared_ptr<AllVersionsMsgsType> const&>
8184
accountProposedSignal_;
82-
TrackableSignal<Subscriber, AllVersionTransactionsType const&> txProposedSignal_;
85+
TrackableSignal<Subscriber, std::shared_ptr<AllVersionsMsgsType> const&> txProposedSignal_;
8386

8487
std::unordered_set<SubscriberPtr> notified_; // Used by slots to prevent double notifications
8588
// if tx contains multiple subscribed accounts

0 commit comments

Comments
 (0)