Page MenuHomePhabricator

rest: Pass in NodeContext to rest_block and use it
ClosedPublic

Authored by PiRK on Jun 8 2022, 15:20.

Details

Reviewers
Fabien
Group Reviewers
Restricted Project
Commits
rABCc3bd9bc2d4b6: rest: Pass in NodeContext to rest_block and use it
Summary

Note: The third commit fixes an issue introduced by the second, when the node context does not have a chainmanager.

rest: Pass in NodeContext to rest_block

https://github.com/bitcoin/bitcoin/pull/21391/commits/3f0893479908ca28d6127c8d0ada30737cb830be

rest: Use existing NodeContext

https://github.com/bitcoin/bitcoin/pull/21391/commits/d7824acdb9b18fe8f151771a83ccae1681f16c66

rest: Add GetChainman function and use it

This is not the cleanest change but:

  1. It fixes the erroneous use of RPC's Ensure*() in rest.cpp, which cause crashes in REST contexts.

    RPC code wraps all calls in a try/except, REST code does not. Ensure*(), being part of RPC, expects that its throw's will get caught by a try/except. But if you use Ensure*() in REST code, since it doesn't have a try/except wrap, a crash will happen.
  1. It is consistent with other functions like GetMemPool.

Someone can probably make this a bit prettier.

https://github.com/bitcoin/bitcoin/pull/21767/commits/9ecade14252ad1972f668d2d2e4ef44fdfcb944a

This is a backport of core#21391 [6 & 7/14]

Test Plan

ninja all check-all

Event Timeline

PiRK requested review of this revision.Jun 8 2022, 15:20
Fabien requested changes to this revision.Jun 8 2022, 16:56
Fabien added a subscriber: Fabien.

Why do you add a parameter which is unused ? This needs to be squashed with whatever commit uses it.
Also it's interesting to see that there is no warning for that

This revision now requires changes to proceed.Jun 8 2022, 16:56
PiRK retitled this revision from rest: Pass in NodeContext to rest_block to rest: Pass in NodeContext to rest_block and use it.
PiRK edited the summary of this revision. (Show Details)

squash D11584 and D11585

Fabien requested changes to this revision.Jun 9 2022, 14:49
Fabien added inline comments.
src/rest.cpp
27 ↗(On Diff #33916)

That's duplicated

218 ↗(On Diff #33916)

Is there a missing backport that explains why this is not located as the same place as the source material ?

This revision now requires changes to proceed.Jun 9 2022, 14:49
src/rest.cpp
218 ↗(On Diff #33916)

OK it's in D11587

remove duplicated include

This revision is now accepted and ready to land.Jun 9 2022, 16:14