This issue is to discuss the 26.0 Release Candidate Testing Guide. If you have any issues with or feedback on the document, please leave a comment here.
Note: this is for feedback on the document, not on Bitcoin Core or on the 26.0 changes. Please see the #28718 for instructions on how to report bug/results.
Thank you for your feedback
The title of this issue should be 26.0 instead of 25.0
Hey @m3dwards thanks for taking this on, the guide looks great so far! I didn't practically run through all the tests myself yet, but left a few tiny nits from a read-through below:
it is recommended that you compile with sqlite(--with-sqlite=yes), so make sure you have installed the sqlite3 dependency.
default is --with-sqlite=auto which will pick up sqlite3 if it's installed, so this isn't particularly required so long as the dep is installed. If sqlite wallets are needed/used in the guide then it probably still makes sense though, as with it configure should fail if it can't find sqlite3 alerting you in the process instead of silently continuing without it
- The RPC prioritisetransaction allows you to "prioritise" a transaction in the mempool by treating it as if it had a higher fee than it actually does. This new RPC allows you to see which transactions have been prioritised and if they are in your mempool.
+ The RPC prioritisetransaction allows miners to "prioritise" a transaction in their mempool for block assembly by treating it as if it had a higher fee than it actually does. This new RPC fetches prioritised transactions from the node's mempool.
Send a transaction to yourself with a 10 sat per vbyte fee
$ bcli -rpcwallet=test-wallet send "{\"$ADDRESS\": 1}" null "unset" 10
Just a general comment: it can be easier to use the -named flag, so this command (and others) could drop a few unneeded params and become:
$ bcli -rpcwallet=test-wallet -named send outputs="{\"$ADDRESS\": 1}" fee_rate=10
edit: I see you have used named for some of the other commands, so nvm.
Import the mempool
$ bcli importmempool /tmp/26-rc-test/regtest/mempool.dat.old
I guess it's the same in practice, but you've use /tmp/ here instead of $DATA_DIR/regtest/mempool.dat.old
- The `true` at end of the previous command instructs the addnode RPC to us V2 transport.
+ The `true` at end of the previous command instructs the addnode RPC to use V2 transport.
@willcl-ark thank you for your review. Addressed all items.
Great guide thanks for writing it!
would it be possible to remove the $ to make the copying of the snippets work directly in terminal?
@D33r-Gee one of the pieces of feedback I received was to differentiate between when it was asking the user to enter a command and when it was showing the result of a command. It was suggested to use $ to make the distinction.
Noted your comment and will see what others think.
@D33r-Gee one of the pieces of feedback I received was to differentiate between when it was asking the user to enter a command and when it was showing the result of a command. It was suggested to use $ to make the distinction.
Noted your comment and will see what others think.
Make sense on the distinction it is definitely a nit, just got used to previous guides (i.e. 25.0 rc) If others find it useful then no worries...
@m3dwards, many thanks for your great work with this. I've run through all the examples in your guide and found it very easy to follow. Just a few nits from my testing below:
In general, I agree with @D33r-Gee that removing the $ would be helpful. My preference would be for making it as easy as possible to enter the correct input(s) to produce the desired output(s), though I can appreciate the desire to distinguish inputs from outputs.
- [macOS zip packaging](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/26.0-Release-Candidate-Testing-Guide#MacOs-zip-packaging) [PR](https://github.com/bitcoin/bitcoin/pull/28432)
+ [macOS zip packaging](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/26.0-Release-Candidate-Testing-Guide#MacOs-zip-packaging) ([PR](https://github.com/bitcoin/bitcoin/pull/28432))
I was able to complete the Regtest testing of importmempool, but was unable to complete Mainnet testing. I do have a Mainnet synced node (Bitcoin Core 25.1.0), but it was hard for me to access it from my compiled instance of bitcoin-26.0rc2. Is the expectation that after compiling from source, a full IBD is completed with bitcoin-26.0rc2 and then the Mainnet tests should be run? If so, that was not immediately clear to me. If not, I was unable to figure out a solution, despite trying for some time.
- Restest Test
+ Regtest Test
- Check mempool, we should transactions
+ Check mempool, we should see transactions
- And check out unspent outputs for the taproot wallet:
+ And check our unspent outputs for the taproot wallet:
Thanks again!
@kashifs thank you for your review, fixed all the found typos and removed the $ signs.
Yes, for the mainnet test of import mempool you would need to be fully synced. I've added a note about that and an extra call to -getinfo to confirm it.
@m3dwards I've followed the 26.0 RC Testing Guide until the very end, many thanks for all the work you've put on this, it has been very easy to follow and very enjoyably actually, I've learnt new concepts and good practices I haven't done before or even thought about.
A few suggestions:
When mention "The PR" or "the PR" on the different sections paragraphs perhaps it could be useful to set the link directly to the "PR" word itself as you did on the Overview section where sections were listed with their PR linked to each.
Is the "Mac OS zip packaging" section needed? As there's nothing especial and instructions are already specified on sections "1. Grab Latest Release Candidate" and "2. Compile Release Candidate". Maybe you can rename it as "Special considerations when run on Mac OS" or something similar, in case you want to mention those 2 paragraphs regarding the permissions and the reminder of shutting down the app.
In the case that user decides to compile from source it's important to mention (perhaps on "2. Compile Release Candidate") that once they clone the repo they would need to checkout the proper version (e.g.: git checkout -b v26.0rc3 tags/v26.0rc3).
On section:
Perhaps can be mentioned that the "usage" field could vary (e.g.: in my case was 1184 instead of 1136).
{
"loaded": true,
"size": 1,
"bytes": 141,
"usage": 1184,
...
On section:
Since this is the largest testing section, could be structured with a numbered list? Some snippets look a bit similar.
On section:
It could be mentioned that would be visually easier to see the connections using bitcoin-qt accessing to the Peers tab on the Node window (main menu->Window->Peers).
A typo on this section:
-Run a re-index up to block 690000. Warning: Sepending on hardware this will take a long time, probably hours.
+Run a re-index up to block 690000. Warning: Depending on hardware this will take a long time, probably hours.
Many thanks again for writing this guide!
Thank you @pablomartin4btc for the detailed comments! I have implemented most of your suggestions.
I didn't number tap miniscript to keep it consistent with the rest of the document and I didn't link every mention of the word PR but made sure in a section the first mention was linked.
Regarding MacOS packaging, what was missing was the justification of why it was in the document which I have now added. (It's because packaging has changed this version).
v26.0 has been tagged. Closing this issue.
Milestone
26.0