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

Add details to supported BTC addresses in omnilock #439

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

Conversation

XuJiandong
Copy link
Contributor

No description provided.

@XuJiandong XuJiandong requested a review from a team as a code owner May 8, 2024 05:24
@XuJiandong XuJiandong requested review from quake and removed request for quake May 8, 2024 05:24
quake
quake previously approved these changes May 8, 2024
@Hanssen0
Copy link

Hanssen0 commented May 8, 2024

Omnilock has not fully supported the P2SH BTC address format, and we can not distinguish P2SH-P2WPKH addresses from other P2SH addresses. Considering this, mentioning P2SH-P2WPKH addresses without warning can cause permanent locking of assets.

We should remove the P2SH-P2WPKH address format or at least explicitly warn developers.

@XuJiandong
Copy link
Contributor Author

XuJiandong commented May 8, 2024

I think it can be mentioned that the omnilock only supports P2SH-P2WPKH but not generic P2SH.

@Hanssen0
Copy link

Hanssen0 commented May 8, 2024

Do you think we need to tell developers that we strongly recommend NOT to use P2SH addresses to avoid mis-locking?

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.

3 participants