Skip to content

Conversation

div72
Copy link
Member

@div72 div72 commented Jan 27, 2023

Testing checklist:
  • Sync from zero.
  • Alerting.
  • Verifying/Signing messages.
  • Sending/Receiving TXs.
  • RPC commands.
    • addmultisigaddress
    • beaconreport
    • dumpwallet
    • importprivkey
    • importwallet
    • sendalert/sendalert2
    • signmessage
    • signrawtransaction
    • verifymessage

(Testing checklist copied from the libsecp256k1 port PR.)

@jamescowens
Copy link
Member

Taking the plunge on this one, eh? :)

@div72
Copy link
Member Author

div72 commented Mar 21, 2023

Just needs key_io tests and testing now.

@div72 div72 force-pushed the cleanup-key-io branch 3 times, most recently from 383162e to a56ec05 Compare March 22, 2023 10:18
@div72 div72 marked this pull request as ready for review March 22, 2023 10:31
@jamescowens jamescowens self-requested a review March 22, 2023 13:01
@jamescowens jamescowens added compatibility deprecation Removed deprecated functionality refactor This is for refactoring (if also an enhancement, use that label too). labels Mar 22, 2023
@jamescowens jamescowens added this to the Miss Piggy milestone Mar 22, 2023
@div72 div72 changed the title refactor, misc: remove CBitcoin(Address|Secret) [1/4] refactor, misc: remove CBitcoin(Address|Secret) Apr 23, 2023
@jamescowens jamescowens modified the milestones: Miss Piggy, Natasha Feb 5, 2024
@div72 div72 marked this pull request as draft June 5, 2024 12:41
@div72 div72 force-pushed the cleanup-key-io branch 4 times, most recently from 6664c70 to 17f4049 Compare June 6, 2024 21:22
@div72 div72 changed the title [1/4] refactor, misc: remove CBitcoin(Address|Secret) refactor, misc: remove CBitcoin(Address|Secret) Jun 6, 2024
div72 and others added 7 commits July 16, 2024 22:41
Rationale:
    With the recent move to libsecp256k1, most of the key managament
    code was refactored. This commit follows up on that by cleaning
    up the usages of CSecret for the removal of CBitcoinSecret
    altogether.

    Mostly based on upstream commit
    bitcoin/bitcoin@dfa23b9.
Rationale:
    This RPC method is a relic of past that's better replaced
    by the dumpwallet/importwallet combo. This commit removes
    them for the CBitcoinSecret removal.
Rationale:
    This commit replaces the usages of CBitcoinSecret with
    new utility methods. This moves base58.h/cpp towards more of a
    pure utility and moves us closer to upstream.

    Loosely based on
    bitcoin/bitcoin@32e69fa.
Rationale:
    With the recent libsecp256k1 port, most of the key code has been
    moved much close to upstream. This commit is the last step on also
    moving key IO code closer to the upstream. This changes also enable
    us to use bech32 addresses without much hassle.

    The changes were also loosely done to drop the CBase58Data class
    which was not chain agnostic in order to support other chains. (Like
    regtest.)
The second element of items of key_io_valid JSON array was pkhash or
scripthash. This commit changes them to be proper serialized scripts as
thats what the key_io_valid expects.

isTestNet flags are also changed to a chain string.
Rationale:
    Sadly some other tests depends on the chain being mainnet without actually
    setting it to mainnet themselves. Which leads to weird test suite
    failures.
@div72 div72 marked this pull request as ready for review July 29, 2024 16:02
@div72
Copy link
Member Author

div72 commented Oct 3, 2024

@jamescowens This is ready for a review as well.

@jamescowens
Copy link
Member

Tested alerting on testnet (sendalert2).
Tested multisig (see attached document)
multisig testing documentation.txt

Copy link
Member

@jamescowens jamescowens left a comment

Choose a reason for hiding this comment

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

ACK cff5c14

@jamescowens jamescowens merged commit b07ab3f into gridcoin-community:development Oct 6, 2024
jamescowens added a commit that referenced this pull request Feb 16, 2025
Added
 - depends: add cross arm64-darwin support #2775 (@div72)
 - build: add missing randomness checks #2772 (@div72)
 - init, registry: Support -clearallregistryhistory startup parameter #2773 (@jamescowens)

Changed
 - build: omit _FORTIFY_SOURCE on debug #2793 (@div72)
 - doc: cmake is required for Windows depends #2791 (@barton2526)
 - CMake: Set maximum supported Boost version #2788 (@CyberTailor)
 - ci: bump to MacOS 13 #2784 (@div72)
 - build/cmake: disable LevelDB tests #2776 (@div72)
 - util: use XDG_STATE_HOME for datadir on Flatpak #2774 (@div72)
 - util, build: Support miniupnp API version 18+ #2771 (@jamescowens)
 - build: explicitly include FindPkgConfig for CMake #2769 (@jamescowens)
 - ci, cd: bump action versions #2763 (@div72)
 - Sync CMake CI #2762 (@CyberTailor)
 - cpid: Modify CPID local hasher to eliminate compiler warnings on 32 bit archs #2760 (@jamescowens)

Removed
 - refactor, misc: remove CBitcoin(Address|Secret) #2634 (@div72)

Fixed
 - poll, gui: Disable choice add button in poll wizard when choice limit is reached #2792 (@jamescowens)
 - diagnose, rpc: fix compilation with boost 1.87 #2786 (@div72)
 - node: fix build with GCC 15 #2783 (@CyberTailor)
 - ci: use overwrite with brew install on MacOS CMake #2782 (@div72)
 - scraper: Protect access to ConvergedStats.csv.gz with a lock #2779 (@jamescowens)
 - fix build on FreeBSD #2770 (@wilkart)
 - rpc/server: fix removing deprecated commands from command list #2768 (@lrusak)
 - cmake bdb53: disable error for implicit-int with gcc #2767 (@lrusak)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compatibility deprecation Removed deprecated functionality refactor This is for refactoring (if also an enhancement, use that label too).
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants