refactor: Add ReserveTxContext to ApplyViewContext#7661
Conversation
|
This PR has conflicts, please resolve them in order for the PR to be reviewed. |
|
All conflicts have been resolved. Assigned reviewers can now start or resume their review. |
|
This PR has conflicts, please resolve them in order for the PR to be reviewed. |
…om/XRPLF/rippled into mvadari/sponsor/reserve-context
|
All conflicts have been resolved. Assigned reviewers can now start or resume their review. |
|
This PR has conflicts, please resolve them in order for the PR to be reviewed. |
…om/XRPLF/rippled into mvadari/sponsor/reserve-context
|
All conflicts have been resolved. Assigned reviewers can now start or resume their review. |
|
This PR has conflicts, please resolve them in order for the PR to be reviewed. |
…om/XRPLF/rippled into mvadari/sponsor/reserve-context
|
All conflicts have been resolved. Assigned reviewers can now start or resume their review. |
![depthfirst-app[bot]](https://avatars.githubusercontent.com/in/1021002?s=60&v=4){kind=small}
![xrplf-ai-reviewer[bot]](https://avatars.githubusercontent.com/u/67929741?s=60&v=4){kind=small}
| adjustOwnerCount(view, owner, {}, 1, beast::Journal{beast::Journal::getNullSink()}); | ||
| SLE::pointer const page = getPageForToken( | ||
| ctx, owner, nft[sfNFTokenID], [](ApplyViewContext& ctx, AccountID const& owner) { | ||
| adjustOwnerCount(ctx, 1, beast::Journal{beast::Journal::getNullSink()}); |
There was a problem hiding this comment.
Wrong account adjusted — ctx.reserveContext.accountID is the tx submitter, not the NFT recipient. Pass owner explicitly:
| adjustOwnerCount(ctx, 1, beast::Journal{beast::Journal::getNullSink()}); | |
| adjustOwnerCount(ctx.view, owner, nullptr, 1, beast::Journal{beast::Journal::getNullSink()}); |
| { | ||
| auto const accountID = ownerSle->getAccountID(sfAccount); |
There was a problem hiding this comment.
⚪ Severity: LOW
makeFromObject dereferences ownerSle without a null check, unlike makeFromAccount which explicitly throws on null. Multiple callers pass view.peek() results (e.g., OfferHelpers.cpp, CredentialHelpers.cpp) that can return null, risking a null-pointer dereference crash.
Helpful? Add 👍 / 👎
💡 Fix Suggestion
Suggestion: Add a null check for ownerSle before dereferencing it at line 440, consistent with the sibling makeFromAccount function which throws on null at line 422. Add if (!ownerSle) Throw<std::runtime_error>("ReserveContext::makeFromObject : valid owner sle"); before the dereference.
⚠️ Experimental Feature: This code suggestion is automatically generated. Please review carefully.
| { | |
| auto const accountID = ownerSle->getAccountID(sfAccount); | |
| { | |
| if (!ownerSle) | |
| Throw<std::runtime_error>("ReserveContext::makeFromObject : valid owner sle"); | |
| auto const accountID = ownerSle->getAccountID(sfAccount); |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## mvadari/sponsor/apply-view-context #7661 +/- ##
==================================================================
Coverage 82.1% 82.2%
==================================================================
Files 1016 1016
Lines 78233 78295 +62
Branches 9003 9007 +4
==================================================================
+ Hits 64267 64330 +63
+ Misses 13957 13956 -1
Partials 9 9
... and 7 files with indirect coverage changes 🚀 New features to boost your workflow:
|
1 participant
High Level Overview of Change
Context of Change
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)