Skip to content

Conversation

theStack
Copy link
Contributor

@theStack theStack commented May 12, 2022

Information on soft fork status has been moved from the getblockchaininfo RPC to the getdeploymentinfo RPC in #23508. The "softfork" result in getblockchaininfo was still available for 23.0 with the -deprecatedrpc=softforks configuration option, but this can be fully removed now for the next release (24.0).

Copy link
Contributor

@brunoerg brunoerg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Concept ACK

@theStack theStack force-pushed the 202205-rpc-remove_deprecated_softforks_from_getblockchaininfo branch from ff5f15f to a01b92a Compare May 13, 2022 09:49
@theStack
Copy link
Contributor Author

Rebased on master.

Copy link
Contributor

@shaavan shaavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK a01b92a

  • I agree with removing the “softfork” field, which was deprecated in v23.0 from the getblockchaininfo RPC output in v24.0.
  • The code changes look clean and are complete. I was able to compile the Bitcoin core successfully.
  • The added release notes documentation documents the changes done in this PR and explains properly where a user must look if searching for the softfork field.

I am adding a few screenshots to show the proper working of this PR.

  • getblockchainfo RPC works correctly:
    1

  • getblockchaininfo fails when --deprecatedrpc=softforks is used
    2

  • getdeploymentinfo RPC works correctly.
    Screenshot from 2022-05-16 19-45-26

Copy link
Contributor

@ajtowns ajtowns left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK a01b92a

- The `-deprecatedrpc=softforks` configuration option has been removed. The
RPC `getblockchaininfo` no longer returns the `softforks` field, which was
previously deprecated in 23.0. (#23508) Information on soft fork status is
now only available via the `getdeploymentinfo` RPC.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should reference this PR? ie .... RPC. (#25114)

@ajtowns
Copy link
Contributor

ajtowns commented May 16, 2022

@shaavan "getblockchaininfo fails when --deprecatedrpc=softforks is used" -- the -deprecatedrpc=softforks flag should be passed to bitcoind, not bitcoin-cli

@maflcko maflcko merged commit 8270740 into bitcoin:master May 17, 2022
@theStack theStack deleted the 202205-rpc-remove_deprecated_softforks_from_getblockchaininfo branch May 17, 2022 09:21
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request May 28, 2022
…tblockchaininfo

a01b92a doc: add release notes about removal of the `deprecatedrpc=softforks` flag (Sebastian Falbesoner)
8c5533c rpc: remove deprecated "softforks" field from getblockchaininfo (Sebastian Falbesoner)

Pull request description:

  Information on soft fork status has been moved from the `getblockchaininfo` RPC to the `getdeploymentinfo` RPC in bitcoin#23508. The "softfork" result in `getblockchaininfo` was still available for 23.0 with the `-deprecatedrpc=softforks` configuration option, but this can be fully removed now for the next release (24.0).

ACKs for top commit:
  shaavan:
    ACK a01b92a
  ajtowns:
    ACK a01b92a

Tree-SHA512: 692d9d02fdf0b3c18376644a85b24b57efebf612738084c01ef47d47e41861e773688613a808e81f10ab6eec340de00eef96987a1e34d612aaf7f0a0b134d89e
Roasbeef added a commit to Roasbeef/lnd that referenced this pull request Aug 6, 2022
In this commit, we add a check during normal node construction to see if
the backend node supports Taproot. If it doesn't, then we want to
shutdown and force the user to take note.

To check if the node supports Taproot, we'll first try the normal
getblockchaininfo call. If this works, cool, then we can rely on the
value. If it doesn't, then we'll fall back to the getdeploymentinfo call
which was added in a recent version of bitcoind [1]. Newer versions of
bitcoind might also have this call, and the getblockchaininfo call, but
not actually populate the softforks field [2]. In this case, we'll fall
back, and we also account for the case when the getblockchaininfo RPC is
removed all together.

[1]: bitcoin/bitcoin#23508
[2]: bitcoin/bitcoin#25114

Fixes lightningnetwork#6773
Roasbeef added a commit to Roasbeef/lnd that referenced this pull request Aug 13, 2022
In this commit, we add a check during normal node construction to see if
the backend node supports Taproot. If it doesn't, then we want to
shutdown and force the user to take note.

To check if the node supports Taproot, we'll first try the normal
getblockchaininfo call. If this works, cool, then we can rely on the
value. If it doesn't, then we'll fall back to the getdeploymentinfo call
which was added in a recent version of bitcoind [1]. Newer versions of
bitcoind might also have this call, and the getblockchaininfo call, but
not actually populate the softforks field [2]. In this case, we'll fall
back, and we also account for the case when the getblockchaininfo RPC is
removed all together.

[1]: bitcoin/bitcoin#23508
[2]: bitcoin/bitcoin#25114

Fixes lightningnetwork#6773
@bitcoin bitcoin locked and limited conversation to collaborators May 17, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants