fix some trace apis in hooks to exclude trailing nul characters, more governance hook debugging

This commit is contained in:
Richard Holland
2023-09-12 20:22:52 +00:00
parent db8fc670c5
commit 269909d5f9
4 changed files with 979 additions and 836 deletions

View File

@@ -113,6 +113,16 @@ int64_t hook(uint32_t r)
etxn_reserve(1);
// in debug mode the test case can supply a line number that the hook will then print here.
{
uint8_t ln[2];
if (otxn_param(SBUF(ln), "D", 1) == 2)
{
uint16_t lineno = (((uint16_t)ln[0]) << 8U) + ln[1];
trace_num(SBUF("DBGLN"), lineno);
}
}
int64_t tt = otxn_type();
if (tt != 99) // ttINVOKE only
@@ -549,26 +559,53 @@ int64_t hook(uint32_t r)
// add / change member
uint8_t previous_member[32];
int previous_present = (state(previous_member + 12, 20, &n, 1) == 20);
if (previous_present)
{
trace(SBUF("Previous present==:"), previous_member, 32, 1);
}
if (BUFFER_EQUAL_20((previous_member + 12), (topic_data + 12)))
DONE("Governance: Actioning seat change, but seat already contains the new member.");
if (previous_present && !topic_data_zero)
{
// we will not change member count, we're adding a member and removing a member
trace(SBUF("previous_present && !topic_data_zero"),0,0,0);
// pass
}
else
{
int64_t existing_member = state(0,0, topic_data + 12, 20);
// adjust member count
if (previous_present)
{
trace(SBUF("Previous present:"), previous_member, 32, 1);
int existing_member_moving = existing_member >= 0;
if (existing_member_moving)
trace(SBUF("Governance: Moving existing member to new seat."), 0,0,0);
uint8_t op = ((!previous_present) << 2U) +
(topic_data_zero << 1U) + existing_member_moving;
ASSERT(op != 0b011U && op != 0b111U && op < 8);
// logic table:
// E/!E - seat is empty/filled
// Z/!Z - topic data is zero non zero (zero = member deletion)
// M/!M - topic is an existing member who is moving
//
// E|Z|M
// -+-+-
// 0 0 0 - seat is full, vote is for a member, an existing member is not moving MC
// 0 0 1 - seat is full, vote is for a member, an existing member is moving MC--
// 0 1 0 - seat is full, vote is for deletion, an existing member is not moving MC--
// 0 1 1 - seat is full, vote is for deletion, an existing member is moving (impossible)
// 1 0 0 - seat is empty, vote is for a member, member is not an existing member MC++
// 1 0 1 - seat is empty, vote is for a member, member is an existing member MC
// 1 1 0 - seat is empty, vote is for deletion, not an existing member moving MC
// 1 1 1 - seat is empty, vote is for deletion, an existing member moving (impossible)
TRACEVAR(op);
trace_num(SBUF("E"), !previous_present);
trace_num(SBUF("Z"), topic_data_zero);
trace_num(SBUF("M"), existing_member_moving);
// adjust member count
{
if (op == 0b001U || op == 0b010U)
member_count--;
}
else
else if (op == 0b100U)
member_count++;
TRACEVAR(previous_present);
@@ -581,6 +618,20 @@ int64_t hook(uint32_t r)
ASSERT(state_set(&mc, 1, "MC", 2) == 1);
}
// if an existing member is moving we need to delete them before re-adding them
if (existing_member_moving)
{
// delete the old member
// reverse key
uint8_t m = (uint8_t)existing_member;
ASSERT(state_set(0,0, &m, 1) == 0);
// forward key
ASSERT(state_set(0, 0, topic_data + 12, 20) == 0);
}
// we need to garbage collect all their votes
if (previous_present)
{
@@ -618,25 +669,17 @@ int64_t hook(uint32_t r)
ASSERT(state_set(0,0, SBUF(previous_member)) == 0);
}
}
}
if (topic_data_zero)
{
// delete the old member
// reverse key
ASSERT(state_set(0,0, &n, 1) == 0);
// forward key
ASSERT(state_set(0, 0, topic_data + 12, 20) == 0);
ASSERT(state_set(0, 0, previous_member + 12, 20) == 0);
}
else
if (!topic_data_zero)
{
// if the seat was occupied we need to remove their forward key
if (previous_present)
ASSERT(state_set(0,0, previous_member + 12, 20) == 0);
// add the new member
// reverse key
ASSERT(state_set(topic_data + 12, 20, &n, 1) == 20);

View File

@@ -1147,12 +1147,20 @@ DEFINE_HOOK_FUNCTION(
if (read_len > 128)
read_len = 128;
if (read_len > 0)
while (read_len > 0)
{
// skip \0 if present at the end
if (*((const char*)memory + read_ptr + read_len - 1) == '\0')
read_len--;
if (read_len == 0)
break;
j.trace()
<< "HookTrace[" << HC_ACC() << "]: "
<< std::string_view((const char*)memory + read_ptr, read_len)
<< " " << number;
<< ": " << number;
return 0;
}
@@ -1193,6 +1201,11 @@ DEFINE_HOOK_FUNCTION(
{
memcpy(output, memory + mread_ptr, mread_len);
out_len += mread_len;
// skip \0's
if (output[out_len-1] == '\0')
out_len--;
output[out_len++] = ':';
output[out_len++] = ' ';
}
@@ -4435,12 +4448,16 @@ DEFINE_HOOK_FUNCTION(
if (read_len > 128)
read_len = 128;
// omit \0 if present
if (read_len > 0 && *((const char*)memory + read_ptr + read_len - 1) == '\0')
read_len--;
if (float1 == 0)
{
j.trace()
<< "HookTrace[" << HC_ACC() << "]:"
<< (read_len == 0 ? "" : std::string_view((const char*)memory + read_ptr, read_len))
<< " Float 0*10^(0) <ZERO>";
<< ": Float 0*10^(0) <ZERO>";
return 0;
}
@@ -4452,14 +4469,14 @@ DEFINE_HOOK_FUNCTION(
j.trace()
<< "HookTrace[" << HC_ACC() << "]:"
<< (read_len == 0 ? "" : std::string_view((const char*)memory + read_ptr, read_len))
<< " Float <INVALID>";
<< ": Float <INVALID>";
return 0;
}
j.trace()
<< "HookTrace[" << HC_ACC() << "]:"
<< (read_len == 0 ? "" : std::string_view((const char*)memory + read_ptr, read_len))
<< " Float " << (neg ? "-" : "") << man << "*10^(" << exp << ")";
<< ": Float " << (neg ? "-" : "") << man << "*10^(" << exp << ")";
return 0;
HOOK_TEARDOWN();

File diff suppressed because it is too large Load Diff

View File

@@ -388,6 +388,7 @@ struct XahauGenesis_test : public beast::unit_test::suite
auto const m18 = Account("m18");
auto const m19 = Account("m19");
auto const m20 = Account("m20");
auto const m21 = Account("m21");
auto vecFromAcc = [](Account const& acc) -> std::vector<uint8_t>
{
@@ -411,7 +412,7 @@ struct XahauGenesis_test : public beast::unit_test::suite
*/
env.fund(XRP(10000), alice, bob, carol, david, edward,
m6, m7, m8, m9, m10, m11, m12, m13, m14, m15, m16, m17, m18, m19, m20);
m6, m7, m8, m9, m10, m11, m12, m13, m14, m15, m16, m17, m18, m19, m20, m21);
env.close();
@@ -421,6 +422,7 @@ struct XahauGenesis_test : public beast::unit_test::suite
setupGov(env, initial_members_ids);
auto vote = [&](
uint16_t lineno,
Account const& acc,
char topic1,
std::optional<char> topic2,
@@ -444,12 +446,24 @@ struct XahauGenesis_test : public beast::unit_test::suite
txn[jss::HookParameters][1u][jss::HookParameter][jss::HookParameterName] =
"56"; // 'V'
std::string strData;
strData.reserve(data.size() << 1U);
for (uint8_t c : data)
strData += charToHex(c);
{
std::string strData;
strData.reserve(data.size() << 1U);
for (uint8_t c : data)
strData += charToHex(c);
txn[jss::HookParameters][1u][jss::HookParameter][jss::HookParameterValue] = strData;
txn[jss::HookParameters][1u][jss::HookParameter][jss::HookParameterValue] = strData;
}
// add a line number for debugging
txn[jss::HookParameters][2u] = Json::objectValue;
txn[jss::HookParameters][2u][jss::HookParameter][jss::HookParameterName] = "44"; // D
{
std::string strData;
strData += charToHex((uint8_t)(lineno >> 8U));
strData += charToHex((uint8_t)(lineno & 0xFFU));
txn[jss::HookParameters][2u][jss::HookParameter][jss::HookParameterValue] = strData;
}
if (layer)
@@ -536,7 +550,7 @@ struct XahauGenesis_test : public beast::unit_test::suite
// perform and check vote
{
env(vote(acc, topic1, topic2, vote_data), M(lineno), fee(XRP(1)),
env(vote(lineno, acc, topic1, topic2, vote_data), M(lineno), fee(XRP(1)),
shouldFail ? ter(tecHOOK_REJECTED) : ter(tesSUCCESS));
env.close();
auto entry =
@@ -727,7 +741,7 @@ struct XahauGenesis_test : public beast::unit_test::suite
auto doSeatVote = [&](uint64_t lineno, uint8_t seat_no, uint8_t final_count,
AccountID const& candidate, std::optional<AccountID const> previous,
std::vector<Account const*> const& voters)
std::vector<Account const*> const& voters, bool shouldSucceed = true)
{
std::vector<uint8_t> previd { 0,0,0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0,0,0} ;
if (previous)
@@ -738,13 +752,19 @@ struct XahauGenesis_test : public beast::unit_test::suite
int count = voters.size();
for (Account const* voter : voters)
doL1Vote(lineno, *voter, 'S', seat_no, id, previd, --count <= 0);
doL1Vote(lineno, *voter, 'S', seat_no, id, previd, --count <= 0 && shouldSucceed);
{
auto entry = env.le(keylet::hookState(env.master.id(),
uint256::fromVoid(member_count_key), beast::zero));
std::vector<uint8_t> const expected_data {final_count};
BEAST_REQUIRE(!!entry);
if (entry->getFieldVL(sfHookStateData) != expected_data)
std::cout
<< "doSeatVote failed " << lineno <<"L. entry data: `"
<< strHex(entry->getFieldVL(sfHookStateData)) << "` "
<< "expected data: `" << strHex(expected_data) << "`\n";
BEAST_EXPECT(entry->getFieldVL(sfHookStateData) == expected_data);
}
};
@@ -753,7 +773,43 @@ struct XahauGenesis_test : public beast::unit_test::suite
// put edward into seat 0 previously occupied by alice
doSeatVote(__LINE__, 0, 3, edward.id(), {}, {&alice, &bob});
// at this point the governance table looks like
// 0 - edward
// 1 - bob
// 2 - empty
// 3 - empty
// 4 - alice
// fill seats 2,3 with accounts m6 and m7. this should take 2 votes only
// alice's vote alone is not enough
doSeatVote(__LINE__, 2, 3, m6.id(), {}, {&alice}, false);
// edward's vote makes 2/3 which is floor(0.8 * members)
doSeatVote(__LINE__, 2, 4, m6.id(), {}, {&edward});
// now we will need 3 votes to put m7 into seat 3
doSeatVote(__LINE__, 3, 4, m7.id(), {}, {&m6}, false);
doSeatVote(__LINE__, 3, 4, m7.id(), {}, {&alice}, false);
doSeatVote(__LINE__, 3, 5, m7.id(), {}, {&bob}, true);
// now we have: ed, bob, m6, m7, alice
// let's try moving ed to another seat, this should succeed
doSeatVote(__LINE__, 5, 5, edward.id(), {}, {&alice, &bob, &edward, &m7});
// now lets move edward over the top of bob
doSeatVote(__LINE__, 1, 4, edward.id(), bob, {&alice, &bob, &m6, &m7});
// first put bob in where edward was
doSeatVote(__LINE__, 0, 5, bob.id(), {}, {&alice, &edward, &m6});
// now we have: bob, ed, m6, m7, alice
// we're going to fill the remaining seats up to 20
doSeatVote(__LINE__, 5, 6, carol.id(), {}, {&alice, &edward, &m6, &m7});
doSeatVote(__LINE__, 6, 7, david.id(), {}, {&alice, &edward, &m6, &m7});
}