Fix and improve solo stamping #141
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
signrawtransaction was replaced with signrawtransactionwithwallet in Bitcoin Core v0.17.0 Also added debug logging.
petertodd
requested changes
Dec 9, 2023
Sjors
force-pushed
the
2023/11/solo-stamp
branch
from
af3b6ab
to
496884e
Compare
petertodd
reviewed
Mar 12, 2024
Review hint: git diff --color-moved=dimmed-zebra --color-moved-ws=ignore-space-change
Sjors
force-pushed
the
2023/11/solo-stamp
branch
from
496884e
to
640e9c8
Compare
petertodd
reviewed
Mar 12, 2024
|
|
||
| blockhash = None | ||
|
|
||
| logging.info('Waiting for confirmation. This can be interupted and resumed with:\nots stamp --nonce=%s --txid=%s', nonce.hex(), txid[::-1].hex()) |
There was a problem hiding this comment.
This doesn't actually work as intended. You need to run ots stamp -b --nonce=x --txid=x. If the -b is left out stamping happens normally, ignoring the nonce and txid arguments.
Maybe we want --nonce and --txid to force -b on?
There was a problem hiding this comment.
Another issue we have right now is that if you use the txid and nonce options after a block has confirmed, ots fails to find the new block.
Maybe we could query the wallet for the transaction? Or maybe we also need a block height?
If we have all three options, we probably want to do it via a single --resume option that can find the tx even if the txid changes.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently trying to stamp with
--btc-walletwill fail for two reasons:signrawtransactionRPC has been deprecated for yearsThis PR switches to the replacement
signrawtransactionwithwalletRPC. It also skips using the calendars when using--btc-wallet.In addition it adds a
--fee-rateargument so you're not dependent on whatever Bitcoin Core decides.In case the process fails mid way after broadcasting a transaction, as it did before this PR, there's now a way to resume by passing a
--nonceand--txidargument. This has the nice benefit of also making RBF possible.Finally it documents in the README that this functionality exists, but with a warning.
Unrelated, it also adds a
--btc-signetoption, though at least for stamping--btc-testnetworks fine with signet.Fixes #139 and #140.