Skip to content

Remove some signature_versions from coins definitions#1523

Open
mariocynicys wants to merge 2 commits intoGLEECBTC:masterfrom
mariocynicys:patch-1
Open

Remove some signature_versions from coins definitions#1523
mariocynicys wants to merge 2 commits intoGLEECBTC:masterfrom
mariocynicys:patch-1

Conversation

@mariocynicys
Copy link

@mariocynicys mariocynicys commented Sep 25, 2025

Removed signature_version fields from multiple coin configurations.

For FRC, CRNC & MAZA, such config wasn't needed since it's already implied because we have no "fork_id": "0x40" set for them.

For CAS, AVN & LCC, those are kept as is because the signature_version config actually has a purpose. If we remove the signature_version: "base" from there, those coins will default to signature_version: "fork_id" since the "fork_id": "0x40" option is set for those coins.

LCC-segwit configuration looks confusing. The signature version is set to "base" and it has a "fork_id": "0x40" option set, but arguablly, these options are all useless in signing because segwit coins always get signed as "WitnessV0".

Removed 'signature_version' field from multiple coin configurations.

For `FRC`, `CRNC` & `MAZA`, such config wasn't needed since it's already implied because we have no "fork_id" set for them.

For `CAS`, `AVN` & `LCC`, those are kept as is because the signature_version config actually has a purpose. If we remove the `signature_version: "base"` from there, those coins will default to `signature_version: "fork_id"` since the "fork_id" option is set for those coins.

`LCC-segwit` configuration looks confusing. The signature version is set to `"base"` and it has a `"fork_id"` option set, but arguablly, these options are all useless in signing because segwit coins always get signed as `"WitnessV0"`.
make sure to set all `signature_version`s for all `fork_id: 0x40` chains to avoid confusion.

normally, for a `fork_id: 0x40` chain, if the `signature_version` is not set, it defaults to `signature_version: "fork_id"`. this commit just makes this behaviour explicit.
@mariocynicys
Copy link
Author

for LCC-segwit, fork_id: 0x40 is actually needed for the signature. but signature_version isn't used at all since segwit coins auto mask any signing operation as segwit. (i.e. if we set signature_version to any value: fork_id, base, witness_v0, it wouldn't make any difference).

but i will leave LCC-segwit's signature_version just for the sake of explicity since that coin already has a fork_id: 0x40. that's better to not always re-think everytime we see the config how defaults kick in with different fork_id & address_format combinations.

@smk762
Copy link
Collaborator

smk762 commented Sep 30, 2025

@mariocynicys can you please propose testing for this PR? E.g. KDF method & expected outcome etc.

@mariocynicys
Copy link
Author

mariocynicys commented Sep 30, 2025

@smk762

For coins that remove signature: base (FRC, CRNC, MAZA). it is really trivial to deduce that the signature will be base as it was since there is no fork_id provided and the coins aren't segwit. So you can really skip testing those if you want.

For the coins that add explicit signature: fork_id (BCH, tBCH, XEC, XRG). We can test those by enabling the coin and signing any transaction (withdrawing for e.g.) and check if this transaction is valid (you don't have to relay the transaction but only check its validity on an explorer or local electrum/node).

For coins that remove signature: witness_v0 (XEP-segwit). same testing flow as before.
in a later PR in KDF, i will remove the ability to deserialize witness_v0 as it will be detected solely by the address format of that coin (and this is actaully what's don't now in the codebase, we parse witness_v0 and never use it).
that's why we shouldn't have any witness_v0s left in the coins config.

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