Skip to content

fix(utxo): deserialize sapling root for PIVX block headers#2572

Merged
shamardy merged 2 commits intodevfrom
fix/pivx-header-deserialization
Aug 5, 2025
Merged

fix(utxo): deserialize sapling root for PIVX block headers#2572
shamardy merged 2 commits intodevfrom
fix/pivx-header-deserialization

Conversation

@shamardy
Copy link
Copy Markdown
Collaborator

@shamardy shamardy commented Aug 4, 2025

This PR resolves a deserialization failure for PIVX block headers that was causing an UnexpectedEnd error.

The root cause was that the BlockHeader deserializer did not account for the hash_final_sapling_root field, which is present in all current PIVX headers. This caused the stream reader to consume an incorrect number of bytes, leading to a misaligned stream for subsequent headers.

The fix introduces a PIVX variant to CoinVariant and updates the BlockHeader::deserialize function to correctly read the hash_final_sapling_root field when the coin is identified as PIVX, ensuring proper headers parsing.

fixes #2048 (comment)

Updates the BlockHeader deserializer to correctly read the `hash_final_sapling_root` field for PIVX.
@shamardy shamardy requested review from cipig and mariocynicys August 4, 2025 23:46
Copy link
Copy Markdown

@cipig cipig left a comment

Choose a reason for hiding this comment

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

thanks for the fix
calling recover_funds_of_swap with the same uuid that previously failed with "UnexpectedEnd" now shows
"error" : "rpc:198] RPC call failed: lp_swap:1519] saved_swap:130] maker_swap:1700] Too early to refund, wait until 1754357000" which is in 1h

@cipig
Copy link
Copy Markdown

cipig commented Aug 5, 2025

oh, just saw that the swap is from 3 months ago and refund already failed with

      {
         "event" : {
            "data" : {
               "error" : "maker_swap:1315] !maker_coin.send_maker_refunds_payment: utxo_common:1958] HTLC spend fee 100000 is greater than transaction output 70000"
            },
            "type" : "MakerPaymentRefundFailed"
         },
         "timestamp" : 1747550467167
      },
      {
         "event" : {
            "type" : "Finished"
         },
         "timestamp" : 1747550467172
      }

so why does recover say i need to wait?
every time i call recover_funds_of_swap, the timestamp in the error increases

@shamardy
Copy link
Copy Markdown
Collaborator Author

shamardy commented Aug 5, 2025

every time i call recover_funds_of_swap, the timestamp in the error increases

Thanks for finding this. The deserializer assumes a Zcash-style field order, but PIVX places its hash_final_sapling_root field at a different position. This means the bytes fit and deserialization completes without errors, but the data assigned to the fields is corrupted. The bytes for the timestamp are incorrectly read as the sapling root, the bytes for the difficulty are read as the timestamp, and so on. Will fix it.

PIVX block headers with `nVersion` >= 8 include a `hash_final_sapling_root`, but its position in the byte stream is different from other Zcash-style chains. It appears after the `nNonce`, not before `nTime`.

The previous deserialization logic incorrectly assumed the Zcash-style order, leading to data corruption for all subsequent fields (`nTime`, `nBits`, etc.) and causing failures in functions that rely on block timestamps, such as swap refunds.

This commit corrects both the serialization and deserialization implementations for `BlockHeader` to handle the PIVX-specific field order. It also introduces a `pivx_mtp` test to validate the fix and prevent regressions.
@shamardy
Copy link
Copy Markdown
Collaborator Author

shamardy commented Aug 5, 2025

should be fixed @cipig

@cipig
Copy link
Copy Markdown

cipig commented Aug 5, 2025

it now refunded that tx that was supposed to be unrefundable because of HTLC spend fee 100000 is greater than transaction output 70000 3 months ago... that's likely due to other changes to kdf in the last 3 months

be42d8c0-127c-4df5-800b-4de8318e1bf5
{
   "result" : {
      "action" : "RefundedMyPayment",
      "coin" : "PIVX",
      "tx_hash" : "2d69c282ed1f59ade4f0d4f26ef2cdfa67aef29ad728835fddcf1424c27cd316",
      "tx_hex" : "0100000001a246b6c5e5df9e98d0ce9dfc52b8cf0df9a6b1567790096411a5209e19e1e59000000000b6473044022055909a48a42fc424fb2b3ba590a1a798ca8e709047222d6c30c73b2f26fde73b02205bf63cf73483ddc191187135214d4a17008fbc4e0fc3961b200bc25b3cd9814d01514c6b6304f97e2968b175210315d9c51c657ab1be4ae9d3ab6e76a619d3bccfe830d5363fa168424c0d044732ac6782012088a914cd9e7193fbf9b6d75ae7b3026f394f55dd6f4b6c882103a27ba503d8d87636f289090f536d6c724ae76b16861e4241e07131fc52ab4025ac68feffffff01b04f0000000000001976a9141462c3dd3f936d595c9af55978003b27c250441f88ac8f669168"
   }
}

on chain: https://chainz.cryptoid.info/pivx/tx.dws?2d69c282ed1f59ade4f0d4f26ef2cdfa67aef29ad728835fddcf1424c27cd316.htm
fees were higher than output, but ok, nice

Copy link
Copy Markdown
Collaborator

@mariocynicys mariocynicys left a comment

Choose a reason for hiding this comment

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

LGTM

@shamardy shamardy merged commit 6172ba8 into dev Aug 5, 2025
18 of 24 checks passed
@shamardy shamardy deleted the fix/pivx-header-deserialization branch August 5, 2025 18:22
dimxy pushed a commit that referenced this pull request Aug 12, 2025
* dev:
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)
dimxy pushed a commit that referenced this pull request Aug 12, 2025
* dev:
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)

# Conflicts:
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
dimxy pushed a commit that referenced this pull request Aug 12, 2025
* dev:
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)

# Conflicts:
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
dimxy pushed a commit that referenced this pull request Aug 13, 2025
* dev:
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)
dimxy pushed a commit that referenced this pull request Aug 15, 2025
* dev: (24 commits)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (#2454)
  ci(pull-requests): review notification bot (#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (#2538)
  bless clippy (#2560)
  refactor(toolchain): use latest available stable compiler (#2557)
  feat(wallet): implement unified offline private key export API (#2542)
  improve note for docker test start failure (#2550)
  fix(DOCS): add note for macos to fix docker containers startup failure (#2544)
  refactor(toolchain): general stabilization for stable rust (#2528)
  ...

# Conflicts:
#	mm2src/coins/eth.rs
#	mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs
#	mm2src/coins/eth/eth_swap_v2/eth_taker_swap_v2.rs
#	mm2src/coins/eth/eth_tests.rs
#	mm2src/coins/eth/eth_withdraw.rs
#	mm2src/coins/eth/v2_activation.rs
#	mm2src/coins/nft.rs
#	mm2src/coins/qrc20.rs
#	mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/rpcs.rs
#	mm2src/mm2_main/src/rpc/lp_commands/tokens.rs
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
dimxy pushed a commit that referenced this pull request Aug 19, 2025
* dev:
  improvement(`static mut`s): `static mut` removal (#2590)
  fix(orders): set subscription on kickstart and skip GC of own pubkeys (#2597)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (#2580)
  fix(clippy): fix clippy warnings for #2565 (#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (#2564)
  feat(protocol): [0] solana support (#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (#2572)
  improvement(dep-stack): security bumps (#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (#2563)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (#2454)
  ci(pull-requests): review notification bot (#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (#2538)
  bless clippy (#2560)
  refactor(toolchain): use latest available stable compiler (#2557)
  feat(wallet): implement unified offline private key export API (#2542)
  chore(release): v2.3.0-beta (#2284)

# Conflicts:
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
dimxy pushed a commit to dimxy/komodo-defi-framework that referenced this pull request Aug 21, 2025
* dev: (28 commits)
  fix build script failing to find .git/HEAD (GLEECBTC#2601)
  refactor(EVM): rename fn, fix timeouts, add activation req validation (GLEECBTC#2543)
  improvement(`static mut`s): `static mut` removal (GLEECBTC#2590)
  fix(orders): set subscription on kickstart and skip GC of own pubkeys (GLEECBTC#2597)
  fix(ordermatch): ignore loop-back; clear on null root; reject stale keep-alives (GLEECBTC#2580)
  fix(clippy): fix clippy warnings for GLEECBTC#2565 (GLEECBTC#2589)
  fix(Trezor): fix utxo and eth calls due to firmware changes (GLEECBTC#2565)
  fix(utxo): calculate min_trading_vol based on fixed tx fees (GLEECBTC#2564)
  feat(protocol): [0] solana support (GLEECBTC#2586)
  fix(utxo): fix header deserialization; guard AuxPoW (GLEECBTC#2583)
  chore(rust 1.89): make CI clippy/fmt pass (wasm32, all-targets) (GLEECBTC#2581)
  fix(utxo): deserialize sapling root for PIVX block headers (GLEECBTC#2572)
  improvement(dep-stack): security bumps (GLEECBTC#2562)
  fix(utxo): correct block header deserialization for AuxPow and KAWPOW coins (GLEECBTC#2563)
  feat(wallet-connect): impl BTC (UTxO) activation via WalletConnect (GLEECBTC#2499)
  feat(utxo): add new fixed txfee option for DINGO-like coins (GLEECBTC#2454)
  ci(pull-requests): review notification bot (GLEECBTC#2468)
  improvement(walletconnect): return the `pairing_topic` in `new_connection` response (GLEECBTC#2538)
  bless clippy (GLEECBTC#2560)
  refactor(toolchain): use latest available stable compiler (GLEECBTC#2557)
  ...

# Conflicts:
#	mm2src/coins/eth.rs
#	mm2src/coins/eth/eth_rpc.rs
#	mm2src/coins/eth/eth_swap_v2/eth_maker_swap_v2.rs
#	mm2src/coins/eth/eth_swap_v2/eth_taker_swap_v2.rs
#	mm2src/coins/eth/eth_withdraw.rs
#	mm2src/coins/eth/v2_activation.rs
#	mm2src/coins/lightning.rs
#	mm2src/coins/lp_coins.rs
#	mm2src/coins/nft.rs
#	mm2src/coins/qrc20.rs
#	mm2src/coins/siacoin.rs
#	mm2src/coins/tendermint/tendermint_token.rs
#	mm2src/coins/test_coin.rs
#	mm2src/coins/utxo/bch.rs
#	mm2src/coins/utxo/qtum.rs
#	mm2src/coins/utxo/slp.rs
#	mm2src/coins/utxo/utxo_standard.rs
#	mm2src/coins/z_coin.rs
#	mm2src/coins_activation/src/bch_with_tokens_activation.rs
#	mm2src/coins_activation/src/erc20_token_activation.rs
#	mm2src/coins_activation/src/eth_with_token_activation.rs
#	mm2src/coins_activation/src/init_erc20_token_activation.rs
#	mm2src/coins_activation/src/init_token.rs
#	mm2src/coins_activation/src/platform_coin_with_tokens.rs
#	mm2src/coins_activation/src/slp_token_activation.rs
#	mm2src/coins_activation/src/tendermint_with_assets_activation.rs
#	mm2src/coins_activation/src/token.rs
#	mm2src/mm2_main/src/lp_swap.rs
#	mm2src/mm2_main/src/lp_swap/check_balance.rs
#	mm2src/mm2_main/src/lp_swap/maker_swap.rs
#	mm2src/mm2_main/src/lp_swap/max_maker_vol_rpc.rs
#	mm2src/mm2_main/src/lp_swap/swap_v2_rpcs.rs
#	mm2src/mm2_main/src/lp_swap/taker_swap.rs
#	mm2src/mm2_main/src/lp_swap/trade_preimage.rs
#	mm2src/mm2_main/src/rpc/dispatcher/dispatcher.rs
#	mm2src/mm2_main/src/rpc/lp_commands/legacy.rs
#	mm2src/mm2_main/src/rpc/lp_commands/lr_swap.rs
#	mm2src/mm2_main/src/rpc/lp_commands/lr_swap/lr_impl.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/errors.rs
#	mm2src/mm2_main/src/rpc/lp_commands/one_inch/rpcs.rs
#	mm2src/mm2_main/src/rpc/lp_commands/tokens.rs
#	mm2src/mm2_main/tests/docker_tests/docker_tests_common.rs
#	mm2src/mm2_main/tests/docker_tests/docker_tests_inner.rs
#	mm2src/mm2_main/tests/docker_tests/eth_docker_tests.rs
#	mm2src/mm2_main/tests/integration_tests_common/mod.rs
#	mm2src/trading_api/src/one_inch_api/client.rs
dimxy pushed a commit that referenced this pull request Oct 15, 2025
It also introduces a PIVX variant to `CoinVariant` and a `pivx_mtp` test.
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