Skip to content

Commit

Permalink
fix(XLS-38): disallow the same bridge on one chain: (XRPLF#4720)
Browse files Browse the repository at this point in the history
Modify the `XChainBridge` amendment.

Before this patch, two door accounts on the same chain could could own
the same bridge spec (of course, one would have to be the issuer and one
would have to be the locker). While this is silly, it does not violate
any bridge invariants. However, on further review, if we allow this then
the `claim` transactions would need to change. Since it's hard to see a
use case for two doors to own the same bridge, this patch disallows
it. (The transaction will return tecDUPLICATE).
  • Loading branch information
seelabs authored Oct 2, 2023
1 parent 2bb8de0 commit 925aca7
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 11 deletions.
12 changes: 9 additions & 3 deletions src/ripple/app/tx/impl/XChainBridge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1452,13 +1452,19 @@ XChainCreateBridge::preclaim(PreclaimContext const& ctx)
{
auto const account = ctx.tx[sfAccount];
auto const bridgeSpec = ctx.tx[sfXChainBridge];

STXChainBridge::ChainType const chainType =
STXChainBridge::srcChain(account == bridgeSpec.lockingChainDoor());

if (ctx.view.read(keylet::bridge(bridgeSpec, chainType)))
{
return tecDUPLICATE;
auto hasBridge = [&](STXChainBridge::ChainType ct) -> bool {
return ctx.view.exists(keylet::bridge(bridgeSpec, ct));
};

if (hasBridge(STXChainBridge::ChainType::issuing) ||
hasBridge(STXChainBridge::ChainType::locking))
{
return tecDUPLICATE;
}
}

if (!isXRP(bridgeSpec.issue(chainType)))
Expand Down
28 changes: 20 additions & 8 deletions src/test/app/XChain_test.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -472,15 +472,16 @@ struct XChain_test : public beast::unit_test::suite,
mcBob, bridge(mcBob, mcGw["USD"], mcBob, mcGw["USD"])),
ter(temXCHAIN_EQUAL_DOOR_ACCOUNTS));

// Both door accounts are on the same chain is allowed (they likely
// belong to different chains. If they do belong to the same chain, that
// is silly, but doesn't violate any invariants).
// Both door accounts are on the same chain. This is not allowed.
// Although it doesn't violate any invariants, it's not a useful thing
// to do and it complicates the "add claim" transactions.
XEnv(*this)
.tx(create_bridge(
mcAlice, bridge(mcAlice, mcGw["USD"], mcBob, mcBob["USD"])))
.close()
.tx(create_bridge(
mcBob, bridge(mcAlice, mcGw["USD"], mcBob, mcBob["USD"])))
mcBob, bridge(mcAlice, mcGw["USD"], mcBob, mcBob["USD"])),
ter(tecDUPLICATE))
.close();

// Create a bridge on an account with exactly enough balance to
Expand Down Expand Up @@ -622,19 +623,30 @@ struct XChain_test : public beast::unit_test::suite,
Account const a2("a2");
Account const a3("a3");
Account const a4("a4");
Account const a5("a5");
Account const a6("a6");

env.fund(XRP(10000), a1, a2, a3, a4);
env.fund(XRP(10000), a1, a2, a3, a4, a5, a6);
env.close();

// Add a bridge on two different accounts with the same locking and
// issuing assets
env.tx(create_bridge(a1, bridge(a1, GUSD, B, BUSD))).close();
env.tx(create_bridge(a2, bridge(a2, GUSD, B, BUSD))).close();

// Add the exact same bridge to two different accoutns (one must locking
// account and one must be issuing)
// Add the exact same bridge to two different accounts (one locking
// account and one issuing)
env.tx(create_bridge(a3, bridge(a3, GUSD, a4, a4["USD"]))).close();
env.tx(create_bridge(a4, bridge(a3, GUSD, a4, a4["USD"]))).close();
env.tx(create_bridge(a4, bridge(a3, GUSD, a4, a4["USD"])),
ter(tecDUPLICATE))
.close();

// Add the exact same bridge to two different accounts (one issuing
// account and one locking - opposite order from the test above)
env.tx(create_bridge(a5, bridge(a6, GUSD, a5, a5["USD"]))).close();
env.tx(create_bridge(a6, bridge(a6, GUSD, a5, a5["USD"])),
ter(tecDUPLICATE))
.close();

// Test case 1 ~ 5, create bridges
auto const goodBridge1 = bridge(A, GUSD, B, BUSD);
Expand Down

0 comments on commit 925aca7

Please sign in to comment.