Skip to content

Commit 08b02c6

Browse files
authored
Fix amm_info amounts in output map to user input (#1162)
Fixes #1156
1 parent b358649 commit 08b02c6

File tree

2 files changed

+124
-3
lines changed

2 files changed

+124
-3
lines changed

src/rpc/handlers/AMMInfo.cpp

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,9 @@ AMMInfoHandler::process(AMMInfoHandler::Input input, Context const& ctx) const
124124
ammID = sle.getFieldH256(ripple::sfAMMID);
125125
}
126126

127-
auto ammKeylet = ammID != 0 ? keylet::amm(ammID) : keylet::amm(input.issue1, input.issue2);
127+
auto issue1 = input.issue1;
128+
auto issue2 = input.issue2;
129+
auto ammKeylet = ammID != 0 ? keylet::amm(ammID) : keylet::amm(issue1, issue2);
128130
auto const ammBlob = sharedPtrBackend_->fetchLedgerObject(ammKeylet.key, lgrInfo.seq, ctx.yield);
129131

130132
if (not ammBlob)
@@ -137,8 +139,15 @@ AMMInfoHandler::process(AMMInfoHandler::Input input, Context const& ctx) const
137139
if (not accBlob)
138140
return Error{Status{RippledError::rpcACT_NOT_FOUND}};
139141

142+
// If the issue1 and issue2 are not specified, we need to get them from the AMM.
143+
// Otherwise we preserve the mapping of asset1 -> issue1 and asset2 -> issue2 as requested by the user.
144+
if (issue1 == ripple::noIssue() and issue2 == ripple::noIssue()) {
145+
issue1 = amm[sfAsset];
146+
issue2 = amm[sfAsset2];
147+
}
148+
140149
auto const [asset1Balance, asset2Balance] =
141-
getAmmPoolHolds(*sharedPtrBackend_, lgrInfo.seq, ammAccountID, amm[sfAsset], amm[sfAsset2], false, ctx.yield);
150+
getAmmPoolHolds(*sharedPtrBackend_, lgrInfo.seq, ammAccountID, issue1, issue2, false, ctx.yield);
142151
auto const lptAMMBalance = input.accountID
143152
? getAmmLpHolds(*sharedPtrBackend_, lgrInfo.seq, amm, *input.accountID, ctx.yield)
144153
: amm[sfLPTokenBalance];

unittests/rpc/handlers/AMMInfoTests.cpp

Lines changed: 113 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1079,7 +1079,7 @@ TEST_F(RPCAMMInfoHandlerTest, HappyPathWithAuctionSlot)
10791079
});
10801080
}
10811081

1082-
TEST_F(RPCAMMInfoHandlerTest, HappyPathWithAssets)
1082+
TEST_F(RPCAMMInfoHandlerTest, HappyPathWithAssetsMatchingInputOrder)
10831083
{
10841084
backend->setRange(10, 30);
10851085

@@ -1189,3 +1189,115 @@ TEST_F(RPCAMMInfoHandlerTest, HappyPathWithAssets)
11891189
EXPECT_EQ(output.value(), expectedResult);
11901190
});
11911191
}
1192+
1193+
TEST_F(RPCAMMInfoHandlerTest, HappyPathWithAssetsPreservesInputOrder)
1194+
{
1195+
backend->setRange(10, 30);
1196+
1197+
auto const lgrInfo = CreateLedgerInfo(LEDGERHASH, SEQ);
1198+
auto const account1 = GetAccountIDWithString(AMM_ACCOUNT);
1199+
auto const account2 = GetAccountIDWithString(AMM_ACCOUNT2);
1200+
auto const issue1 = ripple::Issue(ripple::to_currency("USD"), account1);
1201+
auto const issue2 = ripple::Issue(ripple::to_currency("JPY"), account2);
1202+
auto const ammKeylet = ripple::keylet::amm(issue1, issue2);
1203+
1204+
// Note: order in the AMM object is different from the input
1205+
auto ammObj = CreateAMMObject(AMM_ACCOUNT, "JPY", AMM_ACCOUNT, "USD", AMM_ACCOUNT2, LP_ISSUE_CURRENCY);
1206+
auto accountRoot = CreateAccountRootObject(AMM_ACCOUNT, 0, 2, 200, 2, INDEX1, 2);
1207+
auto const auctionIssue = ripple::Issue{ripple::Currency{LP_ISSUE_CURRENCY}, account1};
1208+
AMMSetAuctionSlot(
1209+
ammObj, account2, ripple::amountFromString(auctionIssue, "100"), 2, 25 * 3600, {account1, account2}
1210+
);
1211+
accountRoot.setFieldH256(ripple::sfAMMID, ammKeylet.key);
1212+
1213+
ON_CALL(*backend, fetchLedgerBySequence).WillByDefault(Return(lgrInfo));
1214+
ON_CALL(*backend, doFetchLedgerObject(GetAccountKey(account1), testing::_, testing::_))
1215+
.WillByDefault(Return(accountRoot.getSerializer().peekData()));
1216+
ON_CALL(*backend, doFetchLedgerObject(GetAccountKey(account2), testing::_, testing::_))
1217+
.WillByDefault(Return(accountRoot.getSerializer().peekData()));
1218+
ON_CALL(*backend, doFetchLedgerObject(ammKeylet.key, testing::_, testing::_))
1219+
.WillByDefault(Return(ammObj.getSerializer().peekData()));
1220+
1221+
auto static const input = json::parse(fmt::format(
1222+
R"({{
1223+
"asset": {{
1224+
"currency": "USD",
1225+
"issuer": "{}"
1226+
}},
1227+
"asset2": {{
1228+
"currency": "JPY",
1229+
"issuer": "{}"
1230+
}}
1231+
}})",
1232+
AMM_ACCOUNT,
1233+
AMM_ACCOUNT2
1234+
));
1235+
1236+
auto const handler = AnyHandler{AMMInfoHandler{backend}};
1237+
runSpawn([&](auto yield) {
1238+
auto const output = handler.process(input, Context{yield});
1239+
auto expectedResult = json::parse(fmt::format(
1240+
R"({{
1241+
"amm": {{
1242+
"lp_token": {{
1243+
"currency": "{}",
1244+
"issuer": "{}",
1245+
"value": "100"
1246+
}},
1247+
"amount": {{
1248+
"currency": "{}",
1249+
"issuer": "{}",
1250+
"value": "0"
1251+
}},
1252+
"amount2": {{
1253+
"currency": "{}",
1254+
"issuer": "{}",
1255+
"value": "0"
1256+
}},
1257+
"account": "{}",
1258+
"trading_fee": 5,
1259+
"auction_slot": {{
1260+
"time_interval": 20,
1261+
"price": {{
1262+
"currency": "{}",
1263+
"issuer": "{}",
1264+
"value": "100"
1265+
}},
1266+
"discounted_fee": 2,
1267+
"account": "{}",
1268+
"expiration": "2000-01-02T01:00:00+0000",
1269+
"auth_accounts": [
1270+
{{
1271+
"account": "{}"
1272+
}},
1273+
{{
1274+
"account": "{}"
1275+
}}
1276+
]
1277+
}},
1278+
"asset_frozen": false,
1279+
"asset2_frozen": false
1280+
}},
1281+
"ledger_index": 30,
1282+
"ledger_hash": "{}",
1283+
"validated": true
1284+
}})",
1285+
LP_ISSUE_CURRENCY,
1286+
AMM_ACCOUNT,
1287+
"USD",
1288+
AMM_ACCOUNT,
1289+
"JPY",
1290+
AMM_ACCOUNT2,
1291+
AMM_ACCOUNT,
1292+
LP_ISSUE_CURRENCY,
1293+
AMM_ACCOUNT,
1294+
AMM_ACCOUNT2,
1295+
AMM_ACCOUNT,
1296+
AMM_ACCOUNT2,
1297+
LEDGERHASH
1298+
));
1299+
1300+
ASSERT_TRUE(output);
1301+
EXPECT_EQ(output.value(), expectedResult);
1302+
});
1303+
}

0 commit comments

Comments
 (0)