Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test microvm.nix VMs with cirrus-runners #22

Open
wants to merge 303 commits into
base: master
Choose a base branch
from

Conversation

0xB10C
Copy link

@0xB10C 0xB10C commented Nov 15, 2024

I hope it uses the CCACHE_REMOTE_STORAGE, not sure

fanquake and others added 30 commits October 16, 2024 14:50
fa43c4f test: Print CompletedProcess object on error (MarcoFalke)

Pull request description:

  It would be good to know the output on `Error parsing command output`. Otherwise test failures are meaningless: bitcoin#30792 (comment)

  Fix it by just printing the full `CompletedProcess` object.

  Also, use the modern `subprocess.run` to simplify the code.

ACKs for top commit:
  BrandonOdiwuor:
    Code Review ACK fa43c4f
  laanwj:
    This contains some useful information, so ACK fa43c4f

Tree-SHA512: ae7c1cb1f48af2a6feae6d1a5a967c0720f6c6675c1ce20ace7cac18c00f3d4069b8abcc58204855e92ff5303158b9a942bab3b71acae0737768d941a5773c91
79aa828 doc: drop LLVM install instructions (fanquake)

Pull request description:

  Followup from bitcoin#31048.

ACKs for top commit:
  maflcko:
    lgtm ACK 79aa828
  hebasto:
    ACK 79aa828.

Tree-SHA512: 9404845cc9a17f85363ce893addadaaba839b4a37e1e3e64ad4a50eb237ffb78636970480ff2f486ff5bd1b3dba9b85bf3d6654a680b11c6832d17daf6dd6c0a
…d directories

a647d44 doc: update signet documentation related to build directories (Torkel Rogstad)

Pull request description:

  While setting up my own signet I noticed that the binary paths in the documentation for this is out of date, after build artifacts moved to the `build` directory. This PR mimics what happened in bitcoin#30741

ACKs for top commit:
  maflcko:
    lgtm ACK a647d44
  pablomartin4btc:
    ACK a647d44
  tdb3:
    Code review and light test ACK a647d44

Tree-SHA512: ac7c3806e0ff65860c41d7b7bdad538368d8a6d8d289c10f9714804f963bafd3a9658301b6697f110f5462a92826b62770963508d5eebf88bf9a0a8442d9f72d
Add recent changes and fixes for shutdown bugs.

chaincodelabs/libmultiprocess#111: doc: Add internal design section
chaincodelabs/libmultiprocess#113: Add missing include to util.h
chaincodelabs/libmultiprocess#116: shutdown bugfix: destroy RPC system before running cleanup callbacks
chaincodelabs/libmultiprocess#118: shutdown bugfix: Prevent segfault in server if connection is broken during long function call
chaincodelabs/libmultiprocess#119: cmake: avoid libatomic not found error on debian
… win docs

184f12c doc: remove dependency install instructions from win docs (fanquake)

Pull request description:

  This duplicates what is in depends, and is outdated.

  Closes bitcoin#31090.

ACKs for top commit:
  maflcko:
    lgtm ACK 184f12c
  jarolrod:
    ACK 184f12c
  BrandonOdiwuor:
    ACK 184f12c

Tree-SHA512: 089c9ff91c501c22ec1b9d5925a2b8c6cd1ea9ac2b75dd6a8c5fe75cf2f0090d808842cb321017894d2da70a30a87dbc1c4c481771d3c4aba13ce44244fcf392
…n to nanoseconds

cd0edf2 tracing: cast block_connected duration to nanoseconds (0xb10c)

Pull request description:

  When the `validation:block_connected` tracepoint was introduced in 8f37f5c, the connect block duration was passed in microseconds `µs`. By starting to use steady clock in fabf1cd this changed to nanoseconds `ns`. As the test only checked if the duration value is `> 0` as a plausibility check, this went unnoticed. This was detected this when setting up monitoring for block validation time as part of the Great Consensus Cleanup Revival discussion.

  This change casts the duration explicitly to nanoseconds, updates the documentation, and adds a check for an upper bound to the tracepoint interface tests. The upper bound is quite lax as mining the block takes much longer than connecting the empty test block. It's however able to detect a duration passed in an incorrect unit (1000x off).

  A previous version of this PR casted the duration to microseconds `µs` - however, as the last three major releases have had the duration as nanoseconds (and this went unnoticed), we assume that this is the API now and changeing it back to microseconds would break the API again. See also bitcoin#29877 (comment)

ACKs for top commit:
  maflcko:
    re-lgtm ACK cd0edf2
  laanwj:
    re-ACK cd0edf2

Tree-SHA512: 54a1eea0297e01c07c2d071ffafbf97dbd080f763e1dc0014ff086a913b739637c1634b1cf87c90b94a3c2f66006acfaada0414a15769cac761e03bc4aab2a77
to avoid overlooking to update the top-level help if the value of the constant changes.
The question mark (`?`) is interpreted as a wildcard for any single character in Zsh (see https://www.techrepublic.com/article/globbing-wildcard-characters-with-zsh), so `bench_bitcoin -?` will not work on systems using Zsh, such as macOS.

Since `-h` provides equivalent help functionality (as defined in https://github.com/bitcoin/bitcoin/blob/master/src/common/args.cpp#L684-L693), the `benchmarking.md` documentation has been updated to ensure compatibility with macOS.
\
…natives

% build/src/bench/bench_bitcoin -h
[...]
  -help
       Print this help message and exit (also -h or -?)
fac6cfe lint: commit-script-check.sh: echo to stderr (MarcoFalke)

Pull request description:

  This makes it easier to redirect the produced `git diff` on failure. On success, it shouldn't hurt, because the same output is still present, just on stderr.

  Can be tested by introducing a fault in any scripted diff and then calling `commit-script-check.sh HEAD~..HEAD > any_file.txt`. Previously the file contained the full output, now it contains just the diff.

ACKs for top commit:
  TheCharlatan:
    ACK fac6cfe

Tree-SHA512: b4dfad10a4a902729a7ad7533ed0ef86b9e79761083f2ec623d448a551462b268fe04bdba387ca62160dae9ef7b1781e005dec60f18b111d9bfa6b97357108e6
90b4055 Update libmultiprocess library (Ryan Ofsky)

Pull request description:

  Add recent changes and fixes for shutdown bugs.

  chaincodelabs/libmultiprocess#111: doc: Add internal design section
  chaincodelabs/libmultiprocess#113: Add missing include to util.h
  chaincodelabs/libmultiprocess#116: shutdown bugfix: destroy RPC system before running cleanup callbacks
  chaincodelabs/libmultiprocess#118: shutdown bugfix: Prevent segfault in server if connection is broken during long function call
  chaincodelabs/libmultiprocess#119: cmake: avoid libatomic not found error on debian

ACKs for top commit:
  fanquake:
    ACK 90b4055
  TheCharlatan:
    ACK 90b4055

Tree-SHA512: 2c256667f0c16e00bb5a81b2c6d3db103fae211844e32b111bbed673ab2612ad1478e6b3ecd3a867a4e425cfa6e778b67388343626597a8fac800a15cea5e53a
Grouping all db writes into a single atomic write operation.
Speeding up the flow and preventing inconsistent states.
This will be useful in the following-up commit to batch the entire
wallet migration process.
So it can be used within an external db txn context.
The wallet is isolated during migration and reloaded at the end
of the process. There is no benefit on connecting the signals
few lines before unloading the wallet.
Useful to ensure that the in-memory state is updated only
after successfully committing the data to disk.
Preparing it to be used within a broader db txn procedure.
So it can be used within an external db txn context.
Perform a single db write operation for the entire
migration procedure.
Perform a single db write operation for each external wallet
(watch-only and solvables) for the entire migration procedure.
Before, we did not explicity say that both fields
`{source_}mapped_as` (that are optional in getrawaddrman)
will be only available if the asmap config flag is set.

Co-authored-by: Jon Atack <[email protected]>
…orting

86e2a6b [test] A non-standard transaction which is also consensus-invalid should return the consensus error (Antoine Poinsot)
f859ff8 [validation] Improve script check error reporting (dergoegge)

Pull request description:

  An input script might be invalid for multiple reasons. For example, it might fail both a standardness check and a consensus check, which can lead to a `mandatory-script-verify-flag-failed` error being reported that includes the script error string from the standardness failure (e.g. `mandatory-script-verify-flag-failed (Using OP_CODESEPARATOR in non-witness script)`), which is confusing.

ACKs for top commit:
  darosior:
    re-ACK 86e2a6b
  ariard:
    Re-Code Review ACK 86e2a6b
  instagibbs:
    ACK 86e2a6b

Tree-SHA512: 053939107c0bcd6643e9006b2518ddc3a6de47d2c6c66af71a04e8af5cf9ec207f19e54583b7a056efd77571edf5fd4f36c31ebe80d1f0777219c756c055eb42
4d3da08 guix: Enable CET for `glibc` package (Hennadii Stepanov)

Pull request description:

  Pulled from bitcoin#30685. This doesn't need to wait for anything.

ACKs for top commit:
  laanwj:
    ACK 4d3da08
  TheCharlatan:
    ACK 4d3da08

Tree-SHA512: 1f4645971381fd342adec52c826fc0023722519a3e28043c9fe8b64bbc1abad822fcc25a64f3f959e3f3a10f5c119029f4cae13c22bac6badcbec9ae8b501dfc
threewebcode and others added 17 commits November 11, 2024 14:14
Fix typos in miniscript.h
faf2162 refactor: Drop deprecated space in operator""_mst (MarcoFalke)

Pull request description:

  The space is deprecated according to https://en.cppreference.com/w/cpp/language/user_literal#Literal_operators and compilers will start to warn about this. For example, GCC-15 should warn when compiling under C++23, according to https://gcc.gnu.org/onlinedocs/gcc/C_002b_002b-Dialect-Options.html#index-Wdeprecated-literal-operator and Clang-20 will do so by default, according to https://clang.llvm.org/docs/DiagnosticsReference.html#wdeprecated-literal-operator.

  Fix it by removing the unused and deprecated space.

  Also, fix the iwyu include list, while touching the module.

ACKs for top commit:
  TheCharlatan:
    ACK faf2162
  l0rinc:
    ACK faf2162

Tree-SHA512: 888a7b57c91114e1f71b6278fa13783bde16a9b51f4df10ae4b6c7d62bf016d6c022128250e6962b459f66743ddeab774b4109064281654892ecdb5bc11b6be6
5a96767 depends, libevent: Do not install *.pc files and remove patches for them (Hennadii Stepanov)
ffda355 cmake, refactor: Move `HAVE_EVHTTP_...` to `libevent` interface (Hennadii Stepanov)
b619bdc cmake: Revamp `FindLibevent` module (Hennadii Stepanov)

Pull request description:

  This PR generalizes the use of `find_package` / `pkg_check_modules`, prioritizing the former.

  Addresses bitcoin#30903 (comment):
  > We should also follow up with refactoring the libevent module, to more generically use CMake/pkg-config, rather than restricting the CMake usage to `vcpkg`. At that point, we'd likely be able to dump pkg-config for the depends path entirely.

  Similar to bitcoin#30903.

ACKs for top commit:
  fanquake:
    ACK 5a96767

Tree-SHA512: 181020c16ccd2821e718c73f264badcdc5e62980c4a8d9691e759efe2ea00da2326e26308d1dcfdeac01e9e27930428ecace9f36941deee951b751b138d7266c
726cbee doc: correct typos (fanquake)
9fdfb73 doc: fix typos (Afanti)

Pull request description:

  Includes bitcoin#31253.
  Includes bitcoin#31239 (review).
  Fixes remaining lint output.

ACKs for top commit:
  l0rinc:
    ACK 726cbee
  rkrux:
    crACK 726cbee
  tdb3:
    ACK 726cbee

Tree-SHA512: 51978343f11fb5f0c6b824d92dbfc9999952373a9f790ab79ef8750f920f1c020c092ca874c9e39f478d12d85cdadcfd8c63dda0cbb02745bc55fda28d371e4c
…temultisig

83fab32 test: Add combinerawtransaction test to rpc_createmultisig (Ava Chow)

Pull request description:

  The only coverage of combinerawtransaction is in a legacy wallet only test. So also use it in rpc_createmultisig so that this RPC remains tested after the legacy wallet is removed.

  Split from bitcoin#28710

ACKs for top commit:
  maflcko:
    re-ACK 83fab32
  BrandonOdiwuor:
    Re-ACK 83fab32
  Abdulkbk:
    ACK 83fab32
  brunoerg:
    code review ACK 83fab32
  rkrux:
    tACK 83fab32

Tree-SHA512: 383d88ff6c9b54337ed81c714026e527b0fed41d976959fd5c6863b49d0defa4ea13fdc3d984885c86a2b6380825cd66c17842cc31f20fbec4bc42d86aecbbfa
This test would catch regressions where ephemeral
dust checks are being erroneously applied on outputs
that are not actually dust.
Also known as Ephemeral Dust.

We try to ensure that dust is spent in blocks by requiring:
  - ephemeral dust tx is 0-fee
  - ephemeral dust tx only has one dust output
  - If the ephemeral dust transaction has a child,
    the dust is spent by by that child.

0-fee requirement means there is no incentive to mine
a transaction which doesn't have a child bringing its
own fees for the transaction package.
Checks that transactions in mempool with dust
follow expected invariants.
Works a bit harder to get ephemeral dust
transactions into the mempool.
9de9c85 test: enhance p2p_orphan_handling (tdb3)
33af14b test: reduce assert_debug_log reliance (tdb3)

Pull request description:

  Previously, `p2p_orphan_handling` relied on checking the debug log for orphanage changes.  This updates the tests to reduce debug log checking and add checks using `tx_in_orphanage()` and `getorphantxs` introduced in bitcoin#30793.

ACKs for top commit:
  glozow:
    light code review ACK 9de9c85
  rkrux:
    tACK 9de9c85
  danielabrozzoni:
    ACK 9de9c85

Tree-SHA512: b53bf0d66d727c79eab972b736a074bd04ca652afd89d2a50830247f42734c61c4c2fa883fde179560e39469c81d0e7be478e1faa0992d3688d5e04d75c067d7
5c2e291 bench: Add basic CheckEphemeralSpends benchmark (Greg Sanders)
3f6559f Add release note for ephemeral dust (Greg Sanders)
71a6ab4 test: unit test for CheckEphemeralSpends (Greg Sanders)
21d28b2 fuzz: add ephemeral_package_eval harness (Greg Sanders)
127719f test: Add CheckMempoolEphemeralInvariants (Greg Sanders)
e2e30e8 functional test: Add ephemeral dust tests (Greg Sanders)
4e68f90 rpc: disallow in-mempool prioritisation of dusty tx (Greg Sanders)
e1d3e81 policy: Allow dust in transactions, spent in-mempool (Greg Sanders)
04b2714 functional test: Add new -dustrelayfee=0 test case (Greg Sanders)

Pull request description:

  A replacement for bitcoin#29001

  Now that we have 1P1C relay, TRUC transactions and sibling eviction, it makes sense to retarget this feature more narrowly by not introducing a new output type, and simple focusing on the feature of allowing temporary dust in the mempool.

  Users of this can immediately use dust outputs as:
  1. Single keyed anchor (can be shared by multiple parties)
  2. Single unkeyed anchor, ala P2A

  Which is useful when the parent transaction cannot have fees for technical or accounting reasons.

  What I'm calling "keyed" anchors would be used anytime you don't want a third party to be able to run off with the utxo. As a motivating example, in Ark there is the concept of a "forfeit transaction" which spends a "connector output". The connector output would ideally be 0-value, but you would not want that utxo spend by anyone, because this would cause financial loss for the coordinator of the service: https://arkdev.info/docs/learn/concepts#forfeit-transaction

  Note that this specific use-case likely doesn't work as it involves a tree of dust, but the connector idea in general demonstrates how it could be used.

  Another related example is connector outputs in BitVM2: https://bitvm.org/bitvm2.html .

  Note that non-TRUC usage will be impractical unless the minrelay requirement on individual transactions are dropped in general, which should happen post-cluster mempool.

  Lightning Network intends to use this feature post-29.0 if available: lightning/bolts#1171 (comment)

  It's also useful for Ark, ln-symmetry, spacechains, Timeout Trees, and other constructs with large presigned trees or other large-N party smart contracts.

ACKs for top commit:
  glozow:
    reACK 5c2e291 via range-diff. Nothing but a rebase and removing the conflict.
  theStack:
    re-ACK 5c2e291

Tree-SHA512: 88e6a6b3b91dc425de47ccd68b7668c8e98c5683712e892c588f79ad639ae95c665e2d5563dd5e5797983e7542cbd1d4353bc90a7298d45a1843b05a417f09f5
@0xB10C 0xB10C force-pushed the 2024-11-microvm-ci-test branch 6 times, most recently from d0d7cdd to a47617b Compare November 20, 2024 15:49
@0xB10C 0xB10C force-pushed the 2024-11-microvm-ci-test branch from cfb5e5e to 6ed4f24 Compare November 25, 2024 23:42
@0xB10C 0xB10C force-pushed the 2024-11-microvm-ci-test branch from f747610 to 3dc6117 Compare November 27, 2024 01:59
@0xB10C 0xB10C force-pushed the 2024-11-microvm-ci-test branch from 13b55eb to 7240207 Compare November 27, 2024 10:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.