Skip to content

Commit ea17abb

Browse files
yinyiqian1yinyiqian1
authored
fix: Ensure delegate tests do not silently fail with batch (#5476)
The tests that ensure `tfInnerBatchTxn` won't block delegated transactions silently fail in `Delegate_test.cpp`. This change removes these cases from that file and adds them to `Batch_test.cpp` instead where they do not silently fail, because there the batch delegate results are explicitly checked. Moving them to that file further avoids refactoring many helper functions.
1 parent 35a40a8 commit ea17abb

File tree

2 files changed

+141
-124
lines changed

2 files changed

+141
-124
lines changed

‎src/test/app/Batch_test.cpp‎

Lines changed: 141 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3765,6 +3765,8 @@ class Batch_test : public beast::unit_test::suite
37653765
}
37663766

37673767
// delegated non atomic inner (AccountSet)
3768+
// this also makes sure tfInnerBatchTxn won't block delegated AccountSet
3769+
// with granular permission
37683770
{
37693771
test::jtx::Env env{*this, envconfig()};
37703772

@@ -3810,6 +3812,145 @@ class Batch_test : public beast::unit_test::suite
38103812
BEAST_EXPECT(env.balance(alice) == preAlice - XRP(2) - batchFee);
38113813
BEAST_EXPECT(env.balance(bob) == preBob + XRP(2));
38123814
}
3815+
3816+
// delegated non atomic inner (MPTokenIssuanceSet)
3817+
// this also makes sure tfInnerBatchTxn won't block delegated
3818+
// MPTokenIssuanceSet with granular permission
3819+
{
3820+
test::jtx::Env env{*this, envconfig()};
3821+
Account alice{"alice"};
3822+
Account bob{"bob"};
3823+
env.fund(XRP(100000), alice, bob);
3824+
env.close();
3825+
3826+
auto const mptID = makeMptID(env.seq(alice), alice);
3827+
MPTTester mpt(env, alice, {.fund = false});
3828+
env.close();
3829+
mpt.create({.flags = tfMPTCanLock});
3830+
env.close();
3831+
3832+
// alice gives granular permission to bob of MPTokenIssuanceLock
3833+
env(delegate::set(
3834+
alice, bob, {"MPTokenIssuanceLock", "MPTokenIssuanceUnlock"}));
3835+
env.close();
3836+
3837+
auto const seq = env.seq(alice);
3838+
auto const batchFee = batch::calcBatchFee(env, 0, 2);
3839+
3840+
Json::Value jv1;
3841+
jv1[sfTransactionType] = jss::MPTokenIssuanceSet;
3842+
jv1[sfAccount] = alice.human();
3843+
jv1[sfDelegate] = bob.human();
3844+
jv1[sfSequence] = seq + 1;
3845+
jv1[sfMPTokenIssuanceID] = to_string(mptID);
3846+
jv1[sfFlags] = tfMPTLock;
3847+
3848+
Json::Value jv2;
3849+
jv2[sfTransactionType] = jss::MPTokenIssuanceSet;
3850+
jv2[sfAccount] = alice.human();
3851+
jv2[sfDelegate] = bob.human();
3852+
jv2[sfSequence] = seq + 2;
3853+
jv2[sfMPTokenIssuanceID] = to_string(mptID);
3854+
jv2[sfFlags] = tfMPTUnlock;
3855+
3856+
auto const [txIDs, batchID] = submitBatch(
3857+
env,
3858+
tesSUCCESS,
3859+
batch::outer(alice, seq, batchFee, tfAllOrNothing),
3860+
batch::inner(jv1, seq + 1),
3861+
batch::inner(jv2, seq + 2));
3862+
env.close();
3863+
3864+
std::vector<TestLedgerData> testCases = {
3865+
{0, "Batch", "tesSUCCESS", batchID, std::nullopt},
3866+
{1, "MPTokenIssuanceSet", "tesSUCCESS", txIDs[0], batchID},
3867+
{2, "MPTokenIssuanceSet", "tesSUCCESS", txIDs[1], batchID},
3868+
};
3869+
validateClosedLedger(env, testCases);
3870+
}
3871+
3872+
// delegated non atomic inner (TrustSet)
3873+
// this also makes sure tfInnerBatchTxn won't block delegated TrustSet
3874+
// with granular permission
3875+
{
3876+
test::jtx::Env env{*this, envconfig()};
3877+
Account gw{"gw"};
3878+
Account alice{"alice"};
3879+
Account bob{"bob"};
3880+
env.fund(XRP(10000), gw, alice, bob);
3881+
env(fset(gw, asfRequireAuth));
3882+
env.close();
3883+
env(trust(alice, gw["USD"](50)));
3884+
env.close();
3885+
3886+
env(delegate::set(
3887+
gw, bob, {"TrustlineAuthorize", "TrustlineFreeze"}));
3888+
env.close();
3889+
3890+
auto const seq = env.seq(gw);
3891+
auto const batchFee = batch::calcBatchFee(env, 0, 2);
3892+
3893+
auto jv1 = trust(gw, gw["USD"](0), alice, tfSetfAuth);
3894+
jv1[sfDelegate] = bob.human();
3895+
auto jv2 = trust(gw, gw["USD"](0), alice, tfSetFreeze);
3896+
jv2[sfDelegate] = bob.human();
3897+
3898+
auto const [txIDs, batchID] = submitBatch(
3899+
env,
3900+
tesSUCCESS,
3901+
batch::outer(gw, seq, batchFee, tfAllOrNothing),
3902+
batch::inner(jv1, seq + 1),
3903+
batch::inner(jv2, seq + 2));
3904+
env.close();
3905+
3906+
std::vector<TestLedgerData> testCases = {
3907+
{0, "Batch", "tesSUCCESS", batchID, std::nullopt},
3908+
{1, "TrustSet", "tesSUCCESS", txIDs[0], batchID},
3909+
{2, "TrustSet", "tesSUCCESS", txIDs[1], batchID},
3910+
};
3911+
validateClosedLedger(env, testCases);
3912+
}
3913+
3914+
// inner transaction not authorized by the delegating account.
3915+
{
3916+
test::jtx::Env env{*this, envconfig()};
3917+
Account gw{"gw"};
3918+
Account alice{"alice"};
3919+
Account bob{"bob"};
3920+
env.fund(XRP(10000), gw, alice, bob);
3921+
env(fset(gw, asfRequireAuth));
3922+
env.close();
3923+
env(trust(alice, gw["USD"](50)));
3924+
env.close();
3925+
3926+
env(delegate::set(
3927+
gw, bob, {"TrustlineAuthorize", "TrustlineFreeze"}));
3928+
env.close();
3929+
3930+
auto const seq = env.seq(gw);
3931+
auto const batchFee = batch::calcBatchFee(env, 0, 2);
3932+
3933+
auto jv1 = trust(gw, gw["USD"](0), alice, tfSetFreeze);
3934+
jv1[sfDelegate] = bob.human();
3935+
auto jv2 = trust(gw, gw["USD"](0), alice, tfClearFreeze);
3936+
jv2[sfDelegate] = bob.human();
3937+
3938+
auto const [txIDs, batchID] = submitBatch(
3939+
env,
3940+
tesSUCCESS,
3941+
batch::outer(gw, seq, batchFee, tfIndependent),
3942+
batch::inner(jv1, seq + 1),
3943+
// tecNO_DELEGATE_PERMISSION: not authorized to clear freeze
3944+
batch::inner(jv2, seq + 2));
3945+
env.close();
3946+
3947+
std::vector<TestLedgerData> testCases = {
3948+
{0, "Batch", "tesSUCCESS", batchID, std::nullopt},
3949+
{1, "TrustSet", "tesSUCCESS", txIDs[0], batchID},
3950+
{2, "TrustSet", "tecNO_DELEGATE_PERMISSION", txIDs[1], batchID},
3951+
};
3952+
validateClosedLedger(env, testCases);
3953+
}
38133954
}
38143955

38153956
void

‎src/test/app/Delegate_test.cpp‎

Lines changed: 0 additions & 124 deletions
Original file line numberDiff line numberDiff line change
@@ -913,37 +913,6 @@ class Delegate_test : public beast::unit_test::suite
913913
gw, gw["USD"](0), alice, tfSetfAuth | tfFullyCanonicalSig),
914914
delegate::as(bob));
915915
}
916-
917-
// tfInnerBatchTxn won't block delegated transaction
918-
{
919-
Env env(*this);
920-
Account gw{"gw"};
921-
Account alice{"alice"};
922-
Account bob{"bob"};
923-
env.fund(XRP(10000), gw, alice, bob);
924-
env(fset(gw, asfRequireAuth));
925-
env.close();
926-
env(trust(alice, gw["USD"](50)));
927-
env.close();
928-
929-
env(delegate::set(
930-
gw, bob, {"TrustlineAuthorize", "TrustlineFreeze"}));
931-
env.close();
932-
933-
auto const seq = env.seq(gw);
934-
auto const batchFee = batch::calcBatchFee(env, 0, 2);
935-
auto jv1 = trust(gw, gw["USD"](0), alice, tfSetfAuth);
936-
jv1[sfDelegate] = bob.human();
937-
auto jv2 = trust(gw, gw["USD"](0), alice, tfSetFreeze);
938-
jv2[sfDelegate] = bob.human();
939-
940-
// batch::inner will set tfInnerBatchTxn, this should not
941-
// block delegated transaction
942-
env(batch::outer(gw, seq, batchFee, tfAllOrNothing),
943-
batch::inner(jv1, seq + 1),
944-
batch::inner(jv2, seq + 2));
945-
env.close();
946-
}
947916
}
948917

949918
void
@@ -1234,53 +1203,6 @@ class Delegate_test : public beast::unit_test::suite
12341203
env(jt);
12351204
BEAST_EXPECT((*env.le(alice))[sfDomain] == makeSlice(domain));
12361205
}
1237-
1238-
// tfInnerBatchTxn won't block delegated transaction
1239-
{
1240-
Env env(*this);
1241-
Account alice{"alice"};
1242-
Account bob{"bob"};
1243-
env.fund(XRP(10000), alice, bob);
1244-
env.close();
1245-
1246-
env(delegate::set(
1247-
alice, bob, {"AccountDomainSet", "AccountEmailHashSet"}));
1248-
env.close();
1249-
1250-
auto const seq = env.seq(alice);
1251-
auto const batchFee = batch::calcBatchFee(env, 0, 3);
1252-
1253-
auto jv1 = noop(alice);
1254-
std::string const domain1 = "example1.com";
1255-
jv1[sfDomain] = strHex(domain1);
1256-
jv1[sfDelegate] = bob.human();
1257-
jv1[sfSequence] = seq + 1;
1258-
1259-
auto jv2 = noop(alice);
1260-
std::string const domain2 = "example2.com";
1261-
jv2[sfDomain] = strHex(domain2);
1262-
jv2[sfDelegate] = bob.human();
1263-
jv2[sfSequence] = seq + 2;
1264-
1265-
// bob set domain back and add email hash for alice
1266-
auto jv3 = noop(alice);
1267-
std::string const mh("5F31A79367DC3137FADA860C05742EE6");
1268-
jv3[sfDomain] = strHex(domain1);
1269-
jv3[sfEmailHash] = mh;
1270-
jv3[sfDelegate] = bob.human();
1271-
jv3[sfSequence] = seq + 3;
1272-
1273-
// batch::inner will set tfInnerBatchTxn, this should not
1274-
// block delegated transaction
1275-
env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
1276-
batch::inner(jv1, seq + 1),
1277-
batch::inner(jv2, seq + 2),
1278-
batch::inner(jv3, seq + 3));
1279-
env.close();
1280-
1281-
BEAST_EXPECT((*env.le(alice))[sfDomain] == makeSlice(domain1));
1282-
BEAST_EXPECT(to_string((*env.le(alice))[sfEmailHash]) == mh);
1283-
}
12841206
}
12851207

12861208
void
@@ -1411,52 +1333,6 @@ class Delegate_test : public beast::unit_test::suite
14111333
.flags = tfMPTLock | tfFullyCanonicalSig,
14121334
.delegate = bob});
14131335
}
1414-
1415-
// tfInnerBatchTxn won't block delegated transaction
1416-
{
1417-
Env env(*this);
1418-
Account alice{"alice"};
1419-
Account bob{"bob"};
1420-
env.fund(XRP(100000), alice, bob);
1421-
env.close();
1422-
1423-
auto const mptID = makeMptID(env.seq(alice), alice);
1424-
MPTTester mpt(env, alice, {.fund = false});
1425-
env.close();
1426-
mpt.create({.flags = tfMPTCanLock});
1427-
env.close();
1428-
1429-
// alice gives granular permission to bob of MPTokenIssuanceLock
1430-
env(delegate::set(
1431-
alice, bob, {"MPTokenIssuanceLock", "MPTokenIssuanceUnlock"}));
1432-
env.close();
1433-
1434-
auto const seq = env.seq(alice);
1435-
auto const batchFee = batch::calcBatchFee(env, 0, 2);
1436-
1437-
Json::Value jv1;
1438-
jv1[sfTransactionType] = jss::MPTokenIssuanceSet;
1439-
jv1[sfAccount] = alice.human();
1440-
jv1[sfDelegate] = bob.human();
1441-
jv1[sfSequence] = seq + 1;
1442-
jv1[sfMPTokenIssuanceID] = to_string(mptID);
1443-
jv1[sfFlags] = tfMPTLock;
1444-
1445-
Json::Value jv2;
1446-
jv2[sfTransactionType] = jss::MPTokenIssuanceSet;
1447-
jv2[sfAccount] = alice.human();
1448-
jv2[sfDelegate] = bob.human();
1449-
jv2[sfSequence] = seq + 2;
1450-
jv2[sfMPTokenIssuanceID] = to_string(mptID);
1451-
jv2[sfFlags] = tfMPTUnlock;
1452-
1453-
// batch::inner will set tfInnerBatchTxn, this should not
1454-
// block delegated transaction
1455-
env(batch::outer(alice, seq, batchFee, tfAllOrNothing),
1456-
batch::inner(jv1, seq + 1),
1457-
batch::inner(jv2, seq + 2));
1458-
env.close();
1459-
}
14601336
}
14611337

14621338
void

0 commit comments

Comments
 (0)