bugs in state update

This commit is contained in:
Richard Holland
2022-02-11 12:54:57 +00:00
parent f185f6418e
commit 4ea9187352
3 changed files with 27 additions and 28 deletions

View File

@@ -508,7 +508,6 @@ namespace hook
ripple::TER ripple::TER
setHookState( setHookState(
HookResult& hookResult,
ripple::ApplyContext& applyCtx, ripple::ApplyContext& applyCtx,
ripple::AccountID const& acc, ripple::AccountID const& acc,
ripple::uint256 const& ns, ripple::uint256 const& ns,
@@ -527,7 +526,8 @@ namespace hook
ripple::TER ripple::TER
finalizeHookState( finalizeHookState(
std::shared_ptr<HookStateMap>&, std::shared_ptr<HookStateMap>&,
ripple::ApplyContext&); ripple::ApplyContext&,
ripple::uint256 const&);
// if the txn being executed was an emitted txn then this removes it from the emission directory // if the txn being executed was an emitted txn then this removes it from the emission directory
ripple::TER ripple::TER

View File

@@ -1128,7 +1128,7 @@ Transactor::operator()()
// write state if all chains executed successfully // write state if all chains executed successfully
if (result == tesSUCCESS) if (result == tesSUCCESS)
hook::finalizeHookState(stateMap, ctx_); hook::finalizeHookState(stateMap, ctx_, ctx_.tx.getTransactionID());
// write hook results // write hook results
for (auto& sendResult: sendResults) for (auto& sendResult: sendResults)
@@ -1225,14 +1225,14 @@ Transactor::operator()()
// write any state changes if cbak resulted in accept() // write any state changes if cbak resulted in accept()
if (result == tesSUCCESS) if (result == tesSUCCESS)
hook::finalizeHookState(stateMap, ctx_); hook::finalizeHookState(stateMap, ctx_, ctx_.tx.getTransactionID());
// write the final result // write the final result
ripple::TER result = ripple::TER result =
finalizeHookResult(callbackResult, ctx_, success); finalizeHookResult(callbackResult, ctx_, success);
JLOG(j_.trace()) JLOG(j_.trace())
<< "HookInfo[" << HC_ACC() << "]: " << "HookInfo[" << callbackAccountID << "-" <<ctx_.tx.getAccountID(sfAccount) << "]: "
<< "Callback finalizeHookResult = " << "Callback finalizeHookResult = "
<< result; << result;
@@ -1240,7 +1240,7 @@ Transactor::operator()()
catch (std::exception& e) catch (std::exception& e)
{ {
JLOG(j_.fatal()) JLOG(j_.fatal())
<< "HookError[" << callbackAccountID << "HookError[" << callbackAccountID << "-" <<ctx_.tx.getAccountID(sfAccount) << "]: "
<< "]: Callback failure " << e.what(); << "]: Callback failure " << e.what();
} }

View File

@@ -317,7 +317,6 @@ bool hook::canHook(ripple::TxType txType, uint64_t hookOn) {
// assumes the specified acc has already been checked for authoriation (hook grants) // assumes the specified acc has already been checked for authoriation (hook grants)
TER TER
hook::setHookState( hook::setHookState(
HookResult& hookResult,
ripple::ApplyContext& applyCtx, ripple::ApplyContext& applyCtx,
ripple::AccountID const& acc, ripple::AccountID const& acc,
ripple::uint256 const& ns, ripple::uint256 const& ns,
@@ -328,11 +327,7 @@ hook::setHookState(
auto& view = applyCtx.view(); auto& view = applyCtx.view();
auto j = applyCtx.app.journal("View"); auto j = applyCtx.app.journal("View");
auto const sleAccount = auto const sleAccount =
( acc == hookResult.account ? view.peek(ripple::keylet::account(acc));
view.peek(hookResult.accountKeylet) :
view.peek(ripple::keylet::account(acc)));
JLOG(j.trace()) << "HookState[" << acc << "]: hookResult.account = " << hookResult.account;
if (!sleAccount) if (!sleAccount)
return tefINTERNAL; return tefINTERNAL;
@@ -459,7 +454,7 @@ hook::apply(
std::vector<uint8_t>, std::vector<uint8_t>,
std::vector<uint8_t> std::vector<uint8_t>
>> const& hookParamOverrides, >> const& hookParamOverrides,
std::shared_ptr<HookStateMap> stateMap, std::shared_ptr<HookStateMap>& stateMap,
ApplyContext& applyCtx, ApplyContext& applyCtx,
ripple::AccountID const& account, /* the account the hook is INSTALLED ON not always the otxn account */ ripple::AccountID const& account, /* the account the hook is INSTALLED ON not always the otxn account */
bool callback, bool callback,
@@ -686,14 +681,14 @@ set_state_cache(
if (stateMapNs.find(key) == stateMapNs.end()) if (stateMapNs.find(key) == stateMapNs.end())
{ {
stateMapNs[key] = { modified, data }; stateMapNs[key] = { modified, data };
hookCtx.changedStateCount++; hookCtx.result.changedStateCount++;
return; return;
} }
if (modified) if (modified)
{ {
if (!stateMapNs[key].first) if (!stateMapNs[key].first)
hookCtx.changedStateCount++; hookCtx.result.changedStateCount++;
stateMapNs[key].first = true; stateMapNs[key].first = true;
} }
@@ -870,28 +865,33 @@ ripple::TER
hook:: hook::
finalizeHookState( finalizeHookState(
std::shared_ptr<HookStateMap>& stateMap, std::shared_ptr<HookStateMap>& stateMap,
ripple::ApplyContext& applyCtx) ripple::ApplyContext& applyCtx,
ripple::uint256 const& txnID)
{ {
auto const& j = applyCtx.app.journal("View"); auto const& j = applyCtx.app.journal("View");
uint16_t changeCount = 0; uint16_t changeCount = 0;
// write all changes to state, if in "apply" mode // write all changes to state, if in "apply" mode
for (const auto& accEntry : *(hookResult.stateMap)) { for (const auto& accEntry : *(stateMap))
{
const auto& acc = accEntry.first; const auto& acc = accEntry.first;
for (const auto& nsEntry : accEntry.second) { for (const auto& nsEntry : accEntry.second)
{
const auto& ns = nsEntry.first; const auto& ns = nsEntry.first;
for (const auto& cacheEntry : nsEntry.second) { for (const auto& cacheEntry : nsEntry.second)
{
bool is_modified = cacheEntry.second.first; bool is_modified = cacheEntry.second.first;
const auto& key = cacheEntry.first; const auto& key = cacheEntry.first;
const auto& blob = cacheEntry.second.second; const auto& blob = cacheEntry.second.second;
if (is_modified) { if (is_modified)
{
changeCount++; changeCount++;
if (changeCount == 0) // RH TODO: limit the max number of state changes? if (changeCount >= 0xFFFFU) // RH TODO: limit the max number of state changes?
{ {
// overflow // overflow
JLOG(j.warn()) JLOG(j.warn())
<< "HooKError[" << HR_ACC << "]: SetHooKState failed: Too many state changes"; << "HooKError[TX:" << txnID << "]: SetHooKState failed: Too many state changes";
return tecHOOK_REJECTED; return tecHOOK_REJECTED;
} }
@@ -899,22 +899,22 @@ finalizeHookState(
auto slice = Slice(blob.data(), blob.size()); auto slice = Slice(blob.data(), blob.size());
TER result = TER result =
setHookState(hookResult, applyCtx, acc, ns, key, slice); setHookState(applyCtx, acc, ns, key, slice);
if (result != tesSUCCESS) if (result != tesSUCCESS)
{ {
JLOG(j.warn()) JLOG(j.warn())
<< "HookError[" << HR_ACC() << "]: SetHookState failed: " << result << "HookError[TX:" << txnID << "]: SetHookState failed: " << result
<< " Key: " << key << " Key: " << key
<< " Value: " << slice; << " Value: " << slice;
return result;
} }
// ^ should not fail... checks were done before map insert // ^ should not fail... checks were done before map insert
} }
} }
} }
} }
return changeCount; return tesSUCCESS;
} }
ripple::TER ripple::TER
@@ -941,7 +941,7 @@ removeEmissionEntry(ripple::ApplyContext& applyCtx)
false)) false))
{ {
JLOG(j.fatal()) JLOG(j.fatal())
<< "HookError[" << HR_ACC() << "]: ccl tefBAD_LEDGER"; << "HookError[TX:" << tx.getTransactionID() << "]: removeEmissionEntry failed tefBAD_LEDGER";
return tefBAD_LEDGER; return tefBAD_LEDGER;
} }
@@ -958,7 +958,6 @@ finalizeHookResult(
{ {
auto const& j = applyCtx.app.journal("View"); auto const& j = applyCtx.app.journal("View");
uint16_t change_count = 0;
// open views do not modify add/remove ledger entries // open views do not modify add/remove ledger entries
if (applyCtx.view().open()) if (applyCtx.view().open())