Skip to content

feat(abstract-utxo): verify paygo #6445

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

christopher-fong
Copy link
Contributor

TICKET: BTC-2118

@christopher-fong christopher-fong force-pushed the BTC-2118.verify-proof branch 3 times, most recently from 26ae7f1 to 26d42d4 Compare July 16, 2025 18:36
@christopher-fong christopher-fong force-pushed the BTC-2118.verify-proof branch 2 times, most recently from 5b6f7b0 to c7c914b Compare August 1, 2025 01:53
@christopher-fong christopher-fong marked this pull request as ready for review August 1, 2025 04:31
@christopher-fong christopher-fong requested review from a team as code owners August 1, 2025 04:31
Copy link
Contributor

@OttoAllmendinger OttoAllmendinger left a comment

Choose a reason for hiding this comment

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

please squash commits

*/
export function getPayGoVerificationPubkey(network: utxolib.Network): string | undefined {
if (utxolib.isTestnet(network)) {
return Networks.test.bitcoin.paygoAddressAttestationPubkey;
Copy link
Contributor

Choose a reason for hiding this comment

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

it's a bit of a shame that we have to pull in the whole bloated statics library just for these few bytes

it would be better if we simply hardcoded this value in the AbstractUtxo module itself

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree as well, at first I didn't know that abstract-utxo didn't have statics as a dependency, and we are only importing it for the pubkey

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think we should leave it as is since its part of the statics and its where we store constants

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should leave it as is since its part of the statics and its where we store constants

that's not true, there are plenty of constants that we do not store in statics (eg utxolib.networks)

statics should contain pretty minimal definitions of coins

@christopher-fong christopher-fong marked this pull request as draft August 1, 2025 13:27
@christopher-fong christopher-fong force-pushed the BTC-2118.verify-proof branch 2 times, most recently from 7b4548c to ee7e179 Compare August 1, 2025 15:38
@christopher-fong christopher-fong marked this pull request as ready for review August 1, 2025 21:36
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.

2 participants