Skip to content

Conversation

UdjinM6
Copy link

@UdjinM6 UdjinM6 commented Aug 20, 2019

Should handle this internally instead of relying on a hidden rpc.

@UdjinM6 UdjinM6 added this to the 14.1 milestone Aug 20, 2019
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK

@codablock
Copy link

Can you explain this one a bit more? If this fix is correct, it allows us to remove the setbip69enabled RPC

@UdjinM6
Copy link
Author

UdjinM6 commented Aug 23, 2019

What happens here is that if caller did not request some specific change position (nChangePosRequest == -1), then he probably doesn't care about it and thus we can apply bip69. If he does care (which is the case for fundrawtransaction with changePosition specified in options), then we ignore bip69 logic. Good point re removing setbip69enabled rpc 👍 See 193cf22.

@UdjinM6 UdjinM6 added the RPC Some notable changes to RPC params/behaviour/descriptions label Aug 23, 2019
Copy link
Member

@PastaPastaPasta PastaPastaPasta left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 requested a review from codablock August 28, 2019 08:03
Copy link

@codablock codablock left a comment

Choose a reason for hiding this comment

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

utACK

@UdjinM6 UdjinM6 merged commit f1ff148 into dashpay:develop Aug 28, 2019
codablock pushed a commit to codablock/dash that referenced this pull request Nov 18, 2019
* Fix bip69 vs change position issue

* Drop `setbip69enabled` rpc
MIPPL pushed a commit to biblepay/biblepay that referenced this pull request Nov 24, 2019
* commit '8b14adb206d76fbc6307385999a1c512052e93fa': (21 commits)
  [v0.14.0.x] Update release notes with change log (dashpay#3213)
  [v0.14.0.x] Bump version to 0.14.0.4 and draft release notes (dashpay#3203)
  Circumvent BIP69 sorting in fundrawtransaction.py test (dashpay#3100)
  Fix compile issues
  Merge bitcoin#11397: net: Improve and document SOCKS code
  Slightly optimize ApproximateBestSubset and its usage for PS txes (dashpay#3184)
  Update activemn if protx info changed (dashpay#3176)
  Actually update spent index on DisconnectBlock (dashpay#3167)
  Only track last seen time instead of first and last seen time (dashpay#3165)
  Merge bitcoin#17118: build: depends macOS: point --sysroot to SDK (dashpay#3161)
  Simulate BlockConnected/BlockDisconnected for PS caches
  Few fixes related to SelectCoinsGroupedByAddresses (dashpay#3144)
  Various fixes for mixing queues (dashpay#3138)
  Also consider txindex for transactions in AlreadyHave() (dashpay#3126)
  Ignore recent rejects filter for locked txes (dashpay#3124)
  Make orphan TX map limiting dependent on total TX size instead of TX count (dashpay#3121)
  Update/modernize macOS plist (dashpay#3074)
  Fix bip69 vs change position issue (dashpay#3063)
  Partially revert 3061 (dashpay#3150)
  Fix SelectCoinsMinConf to allow instant respends (dashpay#3061)
  ...

# Conflicts:
#	configure.ac
#	doc/man/biblepay-cli.1
#	doc/man/biblepay-qt.1
#	doc/man/biblepay-tx.1
#	doc/man/biblepayd.1
#	doc/release-notes.md
#	src/clientversion.h
#	src/wallet/wallet.cpp
barrystyle pushed a commit to PACGlobalOfficial/PAC that referenced this pull request Jan 22, 2020
* Fix bip69 vs change position issue

* Drop `setbip69enabled` rpc
@UdjinM6 UdjinM6 deleted the fixbip69fund branch November 26, 2020 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RPC Some notable changes to RPC params/behaviour/descriptions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants