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

Disable P2SH #2277

Closed
adamkrellenstein opened this issue Sep 30, 2024 · 3 comments
Closed

Disable P2SH #2277

adamkrellenstein opened this issue Sep 30, 2024 · 3 comments
Labels

Comments

@adamkrellenstein
Copy link
Member

adamkrellenstein commented Sep 30, 2024

Right now v1 forces P2SH encoding, at least for MPMA transactions. Both v1 and v2 should default to multi-sig, since P2SH is unreliable (#2270 #1383). (It's also probably more expensive in many cases, because it uses multiple transactions.) All of this is a stopgap, given we're gonna be doing all encoding using witness data in the near future (cf. #1375).

@adamkrellenstein
Copy link
Member Author

apparently this actually has something to do with memo, memo_in_hex, and the version of bitcoin core / python-bitcoinlib being used...

@adamkrellenstein adamkrellenstein changed the title Default to Multi-Sig Encoding Disable P2SH Oct 1, 2024
@adamkrellenstein
Copy link
Member Author

adamkrellenstein commented Oct 1, 2024

So it turns out that our P2SH encoding no longer works with recent versions of Bitcoin Core. It was the default only when use_old_api=False was specified. This is causing loss of user funds. Here's the test demonstrating that: #2283

In addition, there were two other bugs in the P2SH implementation:

  1. fee_per_kb is required for P2SH #2287
  2. tx sanity check is broken when there is a destination #2288

Plus we also ran into: #2286

@ouziel-slama
Copy link
Contributor

done here #2290

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants