After upgradewallet, fundrawtransaction uses non change address as change

[LeoComandini]

Wallet created with 0.14, later with 0.17 upgraded with upgradewallet.

fundrawtransction uses non change addresses for the change.
The very next getnewaddress call returns the same address used as change.

$ DUMMY=VJL5bwudesLgrLxF4SPVyxvoWrDJhBpKf9YxQgVpMzfuLDSGndbWS832vZmKoV5KhrC2RC4SKzvn2ZbE
$ TXC=$(liquid-cli createrawtransaction "[]" "{\"$DUMMY\":0.0001234}")
$ TXF=$(liquid-cli fundrawtransaction $TXC | jq -r .hex)
$ ADDR=$(liquid-cli getnewaddress)
$ liquid-cli getaddressinfo $ADDR
{
  "address": "VJL8rZ3UwNfEkRACELoAwxYAayY7WowZ7wKNkrD59j9E9EmzPaqv7S9PT7T8h2YDS6Z5jPhxTzmLgdAV",
  "scriptPubKey": "a9144bf2661eff22f1ca558549da01b2d077cc3bc9de87",
  "ismine": true,
  "iswatchonly": false,
  "isscript": true,
  "iswitness": false,
  "script": "witness_v0_keyhash",
  "hex": "00141c2f767cd5ba1d5aa51067e0d6a2ef7589ad0ef3",
  "pubkey": "03e6008971b88faf6cc8078fadb16ed913a6d3c8c7bd67c7966368c019eaba8f91",
  "embedded": {
    "isscript": false,
    "iswitness": true,
    "witness_version": 0,
    "witness_program": "1c2f767cd5ba1d5aa51067e0d6a2ef7589ad0ef3",
    "pubkey": "03e6008971b88faf6cc8078fadb16ed913a6d3c8c7bd67c7966368c019eaba8f91",
    "address": "ex1qrshhvlx4hgw44fgsvlsddgh0wky66rhn9hr480",
    "scriptPubKey": "00141c2f767cd5ba1d5aa51067e0d6a2ef7589ad0ef3"
  },
  "confidential": "VJL8rZ3UwNfEkRACELoAwxYAayY7WowZ7wKNkrD59j9E9EmzPaqv7S9PT7T8h2YDS6Z5jPhxTzmLgdAV",
  "confidential_key": "027a157aa311ff7c70f745652538dcb258cb105ce3ea262baba988fe5348f21742",
  "unconfidential": "Gp76To3Bt3W2ZRhyxVsgGAh3yGkmRgmQuU",
  "label": "",
  "timestamp": 1554228459,
  "hdkeypath": "m/0'/0'/51'",
  "hdseedid": "822d37757b7e1d372c1b425c25f3f06b9066d923",
  "hdmasterkeyid": "822d37757b7e1d372c1b425c25f3f06b9066d923",
  "labels": [
    {
      "name": "",
      "purpose": "receive"
    }
  ]
}

$ liquid-cli decoderawtransaction $TXF
{
  "txid": "02307ff3b287e583c12b7893d33d784afd7b79c8386a890a7ab2cb32ccc64bf6",
  "hash": "02307ff3b287e583c12b7893d33d784afd7b79c8386a890a7ab2cb32ccc64bf6",
  "wtxid": "02307ff3b287e583c12b7893d33d784afd7b79c8386a890a7ab2cb32ccc64bf6",
  "withash": "ab3393f4fb64685a4de07ac89c6903d6a0ba107c87fff33c9a981a7590bae48e",
  "version": 2,
  "size": 294,
  "vsize": 294,
  "weight": 1176,
  "locktime": 0,
  "vin": [
    {
      "txid": "a9b709f3f936d5dc984fbf85fc1f471588f8a3947b0e51fe62eb184324aa08f9",
      "vout": 1,
      "scriptSig": {
        "asm": "",
        "hex": ""
      },
      "is_pegin": false,
      "sequence": 4294967293
    }
  ],
  "vout": [
    {
      "value": 0.00012340,
      "asset": "6f0279e9ed041c3d710a9f57d0c02928416460c4b722ae3457a11eec381c526d",
      "commitmentnonce": "020202020202020202020202020202020202020202020202020202020202020202",
      "commitmentnonce_fully_valid": true,
      "n": 0,
      "scriptPubKey": {
        "asm": "OP_HASH160 0000000000000000000000000000000000000000 OP_EQUAL",
        "hex": "a914000000000000000000000000000000000000000087",
        "reqSigs": 1,
        "type": "scripthash",
        "addresses": [
          "GhBXQEdEh35AtuSNxMzRutcgYg3nkvq5Wb"
        ]
      }
    },
    {
      "value": 0.00075728,
      "asset": "6f0279e9ed041c3d710a9f57d0c02928416460c4b722ae3457a11eec381c526d",
      "commitmentnonce": "027a157aa311ff7c70f745652538dcb258cb105ce3ea262baba988fe5348f21742",
      "commitmentnonce_fully_valid": true,
      "n": 1,
      "scriptPubKey": {
        "asm": "OP_HASH160 4bf2661eff22f1ca558549da01b2d077cc3bc9de OP_EQUAL",
        "hex": "a9144bf2661eff22f1ca558549da01b2d077cc3bc9de87",
        "reqSigs": 1,
        "type": "scripthash",
        "addresses": [
          "Gp76To3Bt3W2ZRhyxVsgGAh3yGkmRgmQuU"
        ]
      }
    },
    {
      "value": 0.00001932,
      "asset": "6f0279e9ed041c3d710a9f57d0c02928416460c4b722ae3457a11eec381c526d",
      "commitmentnonce": "",
      "commitmentnonce_fully_valid": false,
      "n": 2,
      "scriptPubKey": {
        "asm": "",
        "hex": "",
        "type": "fee"
      }
    }
  ]
}

[instagibbs]

Ok I've confirmed the issue. Seems like it's drawing external chain keys from external pool, and adding them to the internal keypool once used? I'll figure it out.

[LeoComandini]

I investigated a bit and the problem seems to be that fundrawtransaction does not remove the change keys from the keypool. This cf82a9e addressed, but it has been removed later. This problem occurs also for non upgraded wallets.

[instagibbs]

That's only a partial issue. Upgrading wallets should get rid of the old keypool as well.

[instagibbs]

The biggest issue here is that it's not drawing from the internal keypool at all.

[instagibbs]

bitcoin/bitcoin#16193 Filed an issue upstream, as I seem to be able to replicate the exact behavior.

[instagibbs]

Eh it turns out the issue is "expected behavior", but there are 0 tests for it.

Investigating the other half of your issue to see if it's related.

[instagibbs]

Turns out other half is our fault. Fix included: #660

[stevenroose]

Just to be clear, the actual reuse only happens after a wallet is migrated from 0.14.* to 0.17.* and upgraded, right?

[instagibbs]

No, due to internal API changes we broke the CReserveKey lifecycle.