Skip to content

Conversation

@w0xlt
Copy link
Contributor

@w0xlt w0xlt commented Aug 5, 2022

This PR includes the output type description in the log. It currently shows the enum position, which is only useful if the reader knows the code.

Master:

Setting spkMan to active: id = 9f..04, type = 3, internal = 0
Setting spkMan to active: id = 3d..21, type = 2, internal = 0
Setting spkMan to active: id = 69..d4, type = 0, internal = 1
Setting spkMan to active: id = 97..ea, type = 1, internal = 1

PR:

Setting spkMan to active: id = 6a..4f, type = bech32m, internal = false
Setting spkMan to active: id = 83..dc, type = legacy, internal = true
Setting spkMan to active: id = 7e..5d, type = p2sh-segwit, internal = true
Setting spkMan to active: id = bd..d2, type = bech32, internal = true
Setting spkMan to active: id = 13...7c, type = bech32m, internal = true

@fanquake fanquake added the Wallet label Aug 5, 2022
@theStack
Copy link
Contributor

theStack commented Aug 5, 2022

Concept ACK

@w0xlt w0xlt force-pushed the log_load_active_script_pubkeyman branch from 78914c2 to b5a762a Compare August 5, 2022 20:20
@theStack
Copy link
Contributor

theStack commented Aug 6, 2022

Master:

Setting spkMan to active: id = 9f..04, type = 3, internal = 0
Setting spkMan to active: id = 3d..21, type = 4, internal = 0
Setting spkMan to active: id = 69..d4, type = 0, internal = 1
Setting spkMan to active: id = 97..ea, type = 1, internal = 1

How could type ever be 4 in master, considering that the OutputType enum class only contains four elements, i.e. has a range of 0-3?

@w0xlt
Copy link
Contributor Author

w0xlt commented Aug 6, 2022

@theStack you are right. In master, the range is 0-3. I got that log from another development branch and thought it was from the master.
I changed the PR description.

Copy link
Contributor

@theStack theStack left a comment

Choose a reason for hiding this comment

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

Code-review ACK b5a762a

@S3RK
Copy link
Contributor

S3RK commented Aug 8, 2022

Code review ACK b5a762a

@fanquake fanquake requested a review from achow101 August 8, 2022 07:28
@achow101
Copy link
Member

achow101 commented Aug 8, 2022

ACK b5a762a

@achow101 achow101 merged commit a478c53 into bitcoin:master Aug 8, 2022
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Aug 8, 2022
…bKeyMan` log

b5a762a wallet: improve `{LoadActive,Deactivate}ScriptPubKeyMan` log (w0xlt)

Pull request description:

  This PR includes the output type description in the log. It currently shows the enum position, which is only useful if the reader knows the code.

  Master:
  ```
  Setting spkMan to active: id = 9f..04, type = 3, internal = 0
  Setting spkMan to active: id = 3d..21, type = 2, internal = 0
  Setting spkMan to active: id = 69..d4, type = 0, internal = 1
  Setting spkMan to active: id = 97..ea, type = 1, internal = 1
  ```

  PR:
  ```
  Setting spkMan to active: id = 6a..4f, type = bech32m, internal = false
  Setting spkMan to active: id = 83..dc, type = legacy, internal = true
  Setting spkMan to active: id = 7e..5d, type = p2sh-segwit, internal = true
  Setting spkMan to active: id = bd..d2, type = bech32, internal = true
  Setting spkMan to active: id = 13...7c, type = bech32m, internal = true

  ```

ACKs for top commit:
  S3RK:
    Code review ACK b5a762a
  achow101:
    ACK b5a762a
  theStack:
    Code-review ACK b5a762a

Tree-SHA512: 5a79706d5452e523b0456fb8435545c6c8e550b6722c0d7966af79011275a97ed97cab297562e031d601aa855118082c5b770af118783b1faaaec0cba9f9ee6a
@w0xlt w0xlt deleted the log_load_active_script_pubkeyman branch August 10, 2022 19:02
@bitcoin bitcoin locked and limited conversation to collaborators Aug 10, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants