rest: Pass in NodeContext to rest_block and use it
ClosedPublic
Actions

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:

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.

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

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

PiRK created this revision.Jun 8 2022, 15:20

Herald added a reviewer: Restricted Project. · View Herald TranscriptJun 8 2022, 15:20

PiRK requested review of this revision.Jun 8 2022, 15:20

teamcity edited the summary of this revision. (Show Details)Jun 8 2022, 15:20

PiRK edited the summary of this revision. (Show Details)Jun 8 2022, 15:21

PiRK added a child revision: D11585: rest: Use existing NodeContext.

Harbormaster completed remote builds in B19304: Diff 33893.Jun 8 2022, 15:35

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

squash D11584 and D11585

PiRK mentioned this in D11585: rest: Use existing NodeContext.Jun 9 2022, 08:28

PiRK added a child revision: D11586: rpc: refactor Ensure* functions.

Harbormaster completed remote builds in B19318: Diff 33916.Jun 9 2022, 08:43

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 ?