diff --git a/src/ripple/app/tx/impl/XChainBridge.cpp b/src/ripple/app/tx/impl/XChainBridge.cpp index 341488e4f2e..7b924ec1636 100644 --- a/src/ripple/app/tx/impl/XChainBridge.cpp +++ b/src/ripple/app/tx/impl/XChainBridge.cpp @@ -1453,19 +1453,14 @@ XChainCreateBridge::preclaim(PreclaimContext const& ctx) auto const account = ctx.tx[sfAccount]; auto const bridgeSpec = ctx.tx[sfXChainBridge]; - // The bridge can't already exist on this ledger, and the bridge for the - // locking chain and issuing chain can't live on the same ledger. - if (ctx.view.read( - keylet::bridge(bridgeSpec, STXChainBridge::ChainType::locking)) || - ctx.view.read( - keylet::bridge(bridgeSpec, STXChainBridge::ChainType::issuing))) + STXChainBridge::ChainType const chainType = + STXChainBridge::srcChain(account == bridgeSpec.lockingChainDoor()); + + if (ctx.view.read(keylet::bridge(bridgeSpec, chainType))) { return tecDUPLICATE; } - STXChainBridge::ChainType const chainType = - STXChainBridge::srcChain(account == bridgeSpec.lockingChainDoor()); - if (!isXRP(bridgeSpec.issue(chainType))) { auto const sleIssuer = diff --git a/src/ripple/protocol/impl/Indexes.cpp b/src/ripple/protocol/impl/Indexes.cpp index 2fb5b669894..3fef856b365 100644 --- a/src/ripple/protocol/impl/Indexes.cpp +++ b/src/ripple/protocol/impl/Indexes.cpp @@ -400,16 +400,13 @@ Keylet bridge(STXChainBridge const& bridge, STXChainBridge::ChainType chainType) { // A door account can support multiple bridges. On the locking chain - // there can only be one bridge per lockingChainIssue. On the issuing chain - // there can only be one bridge per issuingChainIssue. + // there can only be one bridge per lockingChainCurrency. On the issuing + // chain there can only be one bridge per issuingChainCurrency. auto const& issue = bridge.issue(chainType); return { ltBRIDGE, indexHash( - LedgerNameSpace::BRIDGE, - bridge.door(chainType), - issue.account, - issue.currency)}; + LedgerNameSpace::BRIDGE, bridge.door(chainType), issue.currency)}; } Keylet diff --git a/src/ripple/rpc/handlers/LedgerEntry.cpp b/src/ripple/rpc/handlers/LedgerEntry.cpp index e237b97d8ad..3f37f76e513 100644 --- a/src/ripple/rpc/handlers/LedgerEntry.cpp +++ b/src/ripple/rpc/handlers/LedgerEntry.cpp @@ -407,22 +407,13 @@ doLedgerEntry(RPC::JsonContext& context) // block has a larger scope so the `bridge` variable doesn't // need to be an optional. STXChainBridge const bridge(context.params[jss::bridge]); - - std::optional chainType; - for (auto const ct : - {STXChainBridge::ChainType::locking, - STXChainBridge::ChainType::issuing}) - { - if (*account == bridge.door(ct)) - { - chainType.emplace(ct); - break; - } - } - if (!chainType) + STXChainBridge::ChainType const chainType = + STXChainBridge::srcChain( + account == bridge.lockingChainDoor()); + if (account != bridge.door(chainType)) return std::nullopt; - return keylet::bridge(bridge, *chainType); + return keylet::bridge(bridge, chainType); } catch (...) { diff --git a/src/test/app/XChain_test.cpp b/src/test/app/XChain_test.cpp index c5faf990613..8ecc7083366 100644 --- a/src/test/app/XChain_test.cpp +++ b/src/test/app/XChain_test.cpp @@ -554,23 +554,38 @@ struct XChain_test : public beast::unit_test::suite, } void - testXChainOneBridgePerAccount() + testXChainBridgeCreateConstraints() { /** - * One bridge per door account tests. Some related cases are - * tested previously. + * Bridge create constraints tests. + * + * Define the door's bridge asset collection as the collection of all + * the issuing assets for which the door account is on the issuing chain + * and all the locking assets for which the door account is on the + * locking chain. (note: a door account can simultaneously be on an + * issuing and locking chain). A new bridge is not a duplicate as long + * as the new bridge asset collection does not contain any duplicate + * currencies (even if the issuers differ). * * Create bridges: - * Note we only test one side, since one side does not know what - * bridges the other side has. We pick the sidechain. - * | bridge spec | result - * | main side | - * case 1 | A -> B | tesSUCCESS - * case 2 | B <- A | tecDUPLICATE - * case 3 | B -> A | tecDUPLICATE - * case 4 | C <- B | tecDUPLICATE - * case 5 | C <- A | tesSUCCESS - * where A -> B, means A is the locking door and B is the issuing door. + * + *| Owner | Locking | Issuing | Comment | + *| a1 | a1 USD/GW | USD/B | | + *| a2 | a2 USD/GW | USD/B | Same locking & issuing assets | + *| | | | | + *| a3 | a3 USD/GW | USD/a4 | | + *| a4 | a4 USD/GW | USD/a4 | Same bridge, different accounts | + *| | | | | + *| B | A USD/GW | USD/B | | + *| B | A EUR/GW | USD/B | Fail: Same issuing asset | + *| | | | | + *| A | A USD/B | USD/C | | + *| A | A USD/B | EUR/B | Fail: Same locking asset | + *| A | A USD/C | EUR/B | Fail: Same locking asset currency | + *| | | | | + *| A | A USD/GW | USD/B | Fail: Same bridge not allowed | + *| A | B USD/GW | USD/A | Fail: "A" has USD already | + *| B | A EUR/GW | USD/B | Fail: | * * Note that, now from sidechain's point of view, A is both * a local locking door and a foreign locking door on different @@ -587,7 +602,7 @@ struct XChain_test : public beast::unit_test::suite, */ using namespace jtx; - testcase("One bridge per account"); + testcase("Bridge create constraints"); XEnv env(*this, true); auto& A = scAlice; auto& B = scBob; @@ -596,29 +611,44 @@ struct XChain_test : public beast::unit_test::suite, auto BUSD = B["USD"]; auto CUSD = C["USD"]; auto GUSD = scGw["USD"]; + auto AEUR = A["EUR"]; + auto BEUR = B["EUR"]; + auto CEUR = C["EUR"]; + auto GEUR = scGw["EUR"]; + + // Accounts to own single brdiges + Account const a1("a1"); + Account const a2("a2"); + Account const a3("a3"); + Account const a4("a4"); + + env.fund(XRP(10000), a1, a2, a3, a4); + 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) + env.tx(create_bridge(a3, bridge(a3, GUSD, a4, a4["USD"]))).close(); + env.tx(create_bridge(a4, bridge(a3, GUSD, a4, a4["USD"]))).close(); // Test case 1 ~ 5, create bridges - // Issuing doors use USDs issued by them in bridge spec. - // Define the door's bridge asset collecion as the collection of all the - // issuing assets for which the door account is on the issuing chain and - // all the locking assets for which the door account is on the locking - // chain. (note: a door account can simultainious be on an issuing and - // locking chain). A new bridge is not a duplicate as long as the new - // bridge asset collection does not contain any dupicates. auto const goodBridge1 = bridge(A, GUSD, B, BUSD); auto const goodBridge2 = bridge(A, BUSD, C, CUSD); env.tx(create_bridge(B, goodBridge1)).close(); - // A bridge must be unique to the chain, even if it has different door - // accounts (otherwise a bridge spec isn't enough to look up a bridge) - env.tx(create_bridge(A, bridge(A, GUSD, B, BUSD)), ter(tecDUPLICATE)) - .close(); - env.tx(create_bridge(A, bridge(B, GUSD, A, AUSD))).close(); // Issuing asset is the same, this is a duplicate - env.tx(create_bridge(A, bridge(B, CUSD, A, AUSD)), ter(tecDUPLICATE)) + env.tx(create_bridge(B, bridge(A, GEUR, B, BUSD)), ter(tecDUPLICATE)) .close(); env.tx(create_bridge(A, goodBridge2), ter(tesSUCCESS)).close(); // Locking asset is the same - this is a duplicate - env.tx(create_bridge(A, bridge(A, BUSD, B, BUSD)), ter(tecDUPLICATE)) + env.tx(create_bridge(A, bridge(A, BUSD, B, BEUR)), ter(tecDUPLICATE)) + .close(); + // Locking asset is USD - this is a duplicate even tho it has a + // different issuer + env.tx(create_bridge(A, bridge(A, CUSD, B, BEUR)), ter(tecDUPLICATE)) .close(); // Test case 6 and 7, commits @@ -4169,9 +4199,11 @@ struct XChain_test : public beast::unit_test::suite, void run() override { + testXChainBridgeCreateConstraints(); + return; testXChainBridgeExtraFields(); testXChainCreateBridge(); - testXChainOneBridgePerAccount(); + testXChainBridgeCreateConstraints(); testXChainCreateBridgeMatrix(); testXChainModifyBridge(); testXChainCreateClaimID();