fix(chain): Use network kind of TestnetKind in transparent addresses on Regtest

[arya2]

Motivation

The non-finalized transparent transfer state is storing tx ids spending to or receiving from an address by getting addresses from outputs and assigning the node network's kind as the address network kind. This makes it difficult to query address tx ids or balances because Regtest transparent addresses use the same b58 address prefixes as Testnet, and addresses with the Regtest prefix are parsed as Testnet addresses in most of Zebra.

Closes #9161.

Solution

• Avoid setting transparent address network kind as RegtestKind

Related:

• Removes outdated TODO

Tests

Covered by Zaino's zebrad_send_to_transparent test (see note on modifications)

PR Author's Checklist

• The PR name will make sense to users.
• The PR provides a CHANGELOG summary.
• The solution is tested.
• The documentation is up to date.
• The PR has a priority label.

PR Reviewer's Checklist

• The PR Author's checklist is complete.
• The PR resolves the issue.

[AloeareV]

Follow-up Work

Investigate why the test is still panicking (in quick_shield() now).

On the zingolib side, I've tracked the error through to a call to librustzcash/zcash_client_backend::proto::service::compact_tx_streamer_client::CompactTxStreamerClient::send_transaction, which returns the following error on this commit, and is functional on the parent commit.

[/home/aloe/repos/zingo/zingolib/zingolib/src/grpc_connector.rs:362:9] client.send_transaction(request).await = Err(
    Status {
        code: Internal,
        message: "JsonRpcConnector error: Error: Serialization/Deserialization Error: missing field `result` at line 1 column 498",
        metadata: MetadataMap {
            headers: {
                "content-type": "application/grpc",
                "date": "Tue, 28 Jan 2025 19:29:26 GMT",
                "content-length": "0",
            },
        },
        source: None,
    },
)

[pacu]

Reproduced the same on my side.

thread 'wallet_basic::zebrad_send_to_transparent' panicked at integration-tests/tests/wallet_to_validator.rs:173:49:
called `Result::unwrap()` on an `Err` value: CompleteAndBroadcast(Broadcast(Broadcast("Send Error: status: Internal, message: \"JsonRpcConnector error: Error: Serialization/Deserialization Error: missing field `result` at line 1 column 498\", details: [], metadata: MetadataMap { headers: {\"content-type\": \"application/grpc\", \"date\": \"Tue, 28 Jan 2025 21:52:20 GMT\", \"content-length\": \"0\"} }")))
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
test wallet_basic::zebrad_send_to_transparent ... FAILED

it seems that zingolib is expecting something like

{ "result": {....}} 

and got something really different ? (see at line 1 column 498\")

[mpguerra]

Follow-up Work

Investigate why the test is still panicking (in quick_shield() now).

On the zingolib side, I've tracked the error through to a call to librustzcash/zcash_client_backend::proto::service::compact_tx_streamer_client::CompactTxStreamerClient::send_transaction, which returns the following error on this commit, and is functional on the parent commit.

[/home/aloe/repos/zingo/zingolib/zingolib/src/grpc_connector.rs:362:9] client.send_transaction(request).await = Err(
    Status {
        code: Internal,
        message: "JsonRpcConnector error: Error: Serialization/Deserialization Error: missing field `result` at line 1 column 498",
        metadata: MetadataMap {
            headers: {
                "content-type": "application/grpc",
                "date": "Tue, 28 Jan 2025 19:29:26 GMT",
                "content-length": "0",
            },
        },
        source: None,
    },
)

Would this not be because you're using a JsonRpcConnector whereas the return content type is grpc?

[idky137]

I believe this could actually be zebra rejecting the transaction and Zaino not properly converting that error into a tonic status (zingolabs/zaino#143).

Do we have the Zebra log for this behaviour?

It would be interesting to see what the behaviour is when running this test with LWD.

I have dealt with this error (or something very similar) in that past and the Zebra log showed that the transaction was never accepted.

Also when running the old test with Zaino I got this error but when running that test with LWD no error was ever returned to the wallet (even though the transaction was rejected) but the transaction disappeared once the wallet synced the next mined block.

EDIT: The JsonRpcConnector is used to communicate with zebra/zcashd's JsonRPC server. The inner error is showing that the JsonRpcConnector in Zaino is failing to deserialise the response from Zebra, not that zingolib is failing to deserialise the response from Zaino. This is most likely due to Zebra returning an Error instead of a txid.

[idky137]

Here is the actual response from Zebra:

[zaino-fetch/src/jsonrpc/connector.rs:178:13] &body_str = "{\"jsonrpc\":\"2.0\",\"id\":453,\"error\":{\"code\":-25,\"message\":\"failed to validate tx: WtxId(\\\"private\\\"), error: transaction did not pass consensus validation: could not validate nullifiers and anchors on best chain: immature transparent coinbase spend: attempt to spend OutPoint { hash: transaction::Hash(\\\"55cfa4cfa5cb82fb7c253d1e217e5ee65ba6a9b2133597ef6b084dc10a439e57\\\"), index: 0 } at Height(102), but spends are invalid before Height(159), which is 100 blocks after it was created at Height(59)\"}}"

I suspect that this has something to do with zingolib constructing tx with regtest address prefix but Zebra now is expecting testnet prefixes..

[arya2]

Here is the actual response from Zebra:

Thank you.

I suspect that this has something to do with zingolib constructing tx with regtest address prefix but Zebra now is expecting testnet prefixes..

This was my first thought as well, but transparent address prefixes on Regtest are the same as Testnet, and quick_shield() unsurprisingly doesn't seem to be trying to find or spend any shielded inputs.

I don't see any logic for filtering out coinbase outputs with fewer than 100 confirmations in TxMap::get_spendable_transparent_outputs() (from zingolib), I think it may have been working before only because coinbase spends mature when they reach the finalized state and transaction_records_by_id was missing the outputs in the non-finalized state.

Update:

The test passes when this condition is height <= (target_height - 100) instead of height <= target_height, though that could wrongly filter out spendable outputs outside this test, so it'll need a way to identify coinbase outputs.

[arya2]

I believe this could actually be zebra rejecting the transaction and Zaino not properly converting that error into a tonic status (zingolabs/zaino#143).

I haven't used the client side of jsonrpsee yet, but it may be a good option if Zaino's request client needs changes anyway.

Do we have the Zebra log for this behaviour?

They weren't indicating anything unusual for the last failure.

It would be interesting to see what the behaviour is when running this test with LWD.

I have dealt with this error (or something very similar) in that past and the Zebra log showed that the transaction was never accepted.

Also when running the old test with Zaino I got this error but when running that test with LWD no error was ever returned to the wallet (even though the transaction was rejected) but the transaction disappeared once the wallet synced the next mined block.

LWD should now have this same behaviour with Zebra v2.1.0 or later, there was a bug around reporting errors that wasn't fully fixed until #9067.

[idky137]

The test passes when [this condition](https://github.com/zingolabs/zingolib/blob/dev/zingolib/src/wallet/transaction_records_by_id/trait_inputsource.rs#L302) is height <= (target_height - 100) instead of height <= target_height, though that could wrongly filter out spendable outputs outside this test, so it'll need a way to identify coinbase outputs.

Thanks for this! Iv'e suspected for some time that there will be changes required in Zingolib for the move to Zebrad from Zcashd. I'm actually surprised we've got so far without hitting anything like this yet. I'll ask someone with better knowledge of Zingolib than me to have a look at this and work out the fix.

I haven't used the client side of jsonrpsee yet, but it may be a good option if Zaino's request client needs changes anyway.

We are planning to move Zaino over to jsonrpsee soon. I do like the simplicity of reqwest but now we are also adding a jsonrpc server it makes more sense to use an actual jsonrpc library. And the fact that both Zebra and Zallet are already using jsonrpsee.