-
Notifications
You must be signed in to change notification settings - Fork 113
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
fix(chain): Use network kind of TestnetKind
in transparent addresses on Regtest
#9175
base: main
Are you sure you want to change the base?
Conversation
On the zingolib side, I've tracked the error through to a call to
|
Reproduced the same on my side.
it seems that zingolib is expecting something like
and got something really different ? (see |
Would this not be because you're using a JsonRpcConnector whereas the return content type is grpc? |
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. |
Here is the actual response from Zebra:
I suspect that this has something to do with zingolib constructing tx with regtest address prefix but Zebra now is expecting testnet prefixes.. |
Thank you.
This was my first thought as well, but transparent address prefixes on Regtest are the same as Testnet, and I don't see any logic for filtering out coinbase outputs with fewer than 100 confirmations in Update: The test passes when this condition is |
I haven't used the client side of
They weren't indicating anything unusual for the last failure.
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. |
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.
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. |
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
RegtestKind
Related:
Tests
Covered by Zaino's
zebrad_send_to_transparent
test (see note on modifications)PR Author's Checklist
PR Reviewer's Checklist