Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

RXI-1136 allow multi commits with the same claimID #111

Merged
merged 3 commits into from
Jun 18, 2024
Merged

RXI-1136 allow multi commits with the same claimID #111

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a03d3a0
RXI-1136 allow multi commits with the same claimID
oleks-rip Jun 15, 2024
1035a84
RXI-1136 review fixes:
oleks-rip Jun 18, 2024
bea3654
RXI-1136 UT improvement:
oleks-rip Jun 18, 2024
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
19 changes: 9 additions & 10 deletions src/xbwd/federator/Federator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,7 +347,7 @@ Federator::readDBAttests(ChainType ct)
}

txnsInProcessing_[ct].insert(
fmt::format("create: {:x}", createCount));
{fmt::format("create: {:x}", createCount), transID});
if (!autoSubmit_[ct])
continue;

Expand Down Expand Up @@ -458,7 +458,8 @@ Federator::readDBAttests(ChainType ct)
continue;
}

txnsInProcessing_[ct].insert(fmt::format("claim: {:x}", claimID));
txnsInProcessing_[ct].insert(
{fmt::format("claim: {:x}", claimID), transID});
if (!autoSubmit_[ct])
continue;

Expand Down Expand Up @@ -781,17 +782,15 @@ Federator::onEvent(event::XChainCommitDetected const& e)
}

auto const txnIdHex = ripple::strHex(e.txnHash_.begin(), e.txnHash_.end());
auto const res =
txnsInProcessing_[ct].insert(fmt::format("claim: {:x}", e.claimID_));
auto const res = txnsInProcessing_[ct].insert(
{fmt::format("claim: {:x}", e.claimID_), txnIdHex});
if (!res.second)
{
// Already have this transaction
// TODO: Sanity check the claim id and deliveredAmt match
// TODO: Stop historical transaction collection
JLOGV(
j_.error(),
"XChainCommit already present",
jv("event", e.toJson()));
j_.warn(), "XChainCommit already present", jv("event", e.toJson()));
return; // Don't store it again
}

Expand Down Expand Up @@ -991,7 +990,7 @@ Federator::onEvent(event::XChainAccountCreateCommitDetected const& e)

auto const txnIdHex = ripple::strHex(e.txnHash_.begin(), e.txnHash_.end());
auto const res = txnsInProcessing_[ct].insert(
fmt::format("create: {:x}", e.createCount_));
{fmt::format("create: {:x}", e.createCount_), txnIdHex});
if (!res.second)
{
// Already have this transaction
Expand Down Expand Up @@ -1331,15 +1330,15 @@ Federator::onEvent(event::XChainAttestsResult const& e)
if ((e.type_ == xbwd::XChainTxnType::xChainAddClaimAttestation) &&
e.claimID_)
{
cnt = txnsInProcessing_[oct].erase(
cnt = txnsInProcessing_[oct].get<1>().erase(
fmt::format("claim: {:x}", *e.claimID_));
}
else if (
(e.type_ ==
xbwd::XChainTxnType::xChainAddAccountCreateAttestation) &&
e.createCount_)
{
cnt = txnsInProcessing_[oct].erase(
cnt = txnsInProcessing_[oct].get<1>().erase(
fmt::format("create: {:x}", *e.createCount_));
}
else
Expand Down
27 changes: 25 additions & 2 deletions src/xbwd/federator/Federator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,10 @@
#include <ripple/protocol/XChainAttestations.h>

#include <boost/asio.hpp>
#include <boost/multi_index/hashed_index.hpp>
#include <boost/multi_index/identity.hpp>
#include <boost/multi_index/member.hpp>
#include <boost/multi_index_container.hpp>

#include <atomic>
#include <condition_variable>
Expand Down Expand Up @@ -289,6 +293,23 @@ struct AttestedHistoryTx
fromEvent(FederatorEvent const& e);
};

struct TransactionCache
{
std::string id_; // ClaimID or CreateID
std::string tx_; // tx hash of XChainCommit / XChainAccountCreateCommit
// transaction
};

namespace mi = boost::multi_index;
Copy link
Contributor

Choose a reason for hiding this comment

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

This probably shouldn't be in a header file - it has too wide of a scope.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed ae89cf2

typedef mi::multi_index_container<
TransactionCache,
mi::indexed_by<
mi::hashed_unique<
mi::member<TransactionCache, std::string, &TransactionCache::tx_>>,
mi::hashed_non_unique<
mi::member<TransactionCache, std::string, &TransactionCache::id_>>>>
TransactionCacheContainer;

class Federator
{
enum LoopTypes {
Expand Down Expand Up @@ -336,8 +357,10 @@ class Federator
ChainArray<std::vector<SubmissionPtr>> GUARDED_BY(txnsMutex_) errored_;

// Cache of the events added to processing. It is added so as not to read
// the DB. No need for mutex as event processing is synchronized.
ChainArray<std::unordered_set<std::string>> txnsInProcessing_;
// the DB. No need for mutex as event processing is synchronized. Insertion
// check transaction id for uniqueness. Deletion remove all the entry with
// the same claimID
ChainArray<TransactionCacheContainer> txnsInProcessing_;

// "Window" size for sending attestations
// 0 - no "window"
Expand Down