delete(code): remove dead code from zebra-chain

[oxarbitrage]

Motivation

There is some dead code we can remove from zebra-chain "safely" as part of #5446 , however i am not sure if there is more to delete to close the ticket.

Solution

Remove unused code from zebra-chain.

Review

Reviewer Checklist

• Will the PR name make sense to users?
  • Does it need extra CHANGELOG info? (new features, breaking changes, large changes)
• Are the PR labels correct?
• Does the code do what the ticket and PR says?
• How do you know it works? Does it have tests?

[codecov]

Codecov Report

Merging #5464 (000ce18) into main (737fbac) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #5464      +/-   ##
==========================================
+ Coverage   79.11%   79.12%   +0.01%     
==========================================
  Files         308      308              
  Lines       39324    39350      +26     
==========================================
+ Hits        31111    31137      +26     
  Misses       8213     8213              

[teor2345]

I think we can also remove the SpendingKey and derived keys from sprout, sapling, and orchard. As far as I can tell, they are only used in tests.

We also want to remove all the code that is impacted by these known bugs:

• Stop assuming testnet when parsing keys and addresses #4691
• the key parsing panics we found in PR lint(clippy): add unwrap_used lint #4732
• anything else?

Then I guess we can close those tickets? Maybe with a note that the buggy code was removed?

[teor2345]

ERROR: (gcloud.compute.ssh) Could not fetch resource:

• The resource 'projects/zealous-zebra/zones/us-central1-a/instances/sync-past-checkpoint-5464-merge-dee1b9c' was not found

https://github.com/ZcashFoundation/zebra/actions/runs/3308216160/jobs/5461039042

@gustavovalverde any idea why we occasionally lose GCP instances like this?

[teor2345]

@Mergifyio update

[mergify]

update

☑️ Nothing to do

• #commits-behind>0 [:pushpin: update requirement]
• -closed [:pushpin: update requirement]

[mpguerra]

We also want to remove all the code that is impacted by these known bugs:

* #4691

* the key parsing panics we found in PR #4732

* anything else?

Then I guess we can close those tickets? Maybe with a note that the buggy code was removed?

Are any of these issues happening because of cryptographic implementations in zebra-chain that are unused? If not, they would be out of scope of #5446 which is what this PR is trying to fix

[oxarbitrage]

I think we can also remove the SpendingKey and derived keys from sprout, sapling, and orchard. As far as I can tell, they are only used in tests.

The SpendingKey structs for sapling, sprout and orchard are already marked as code that will only compile for tests so auditors should skip testing code.

Do we want to remove the structs, implementations and tests alltogether ? do we really need to do this ?

[teor2345]

We also want to remove all the code that is impacted by these known bugs:

* #4691

* the key parsing panics we found in PR #4732

* anything else?

Are any of these issues happening because of cryptographic implementations in zebra-chain that are unused?

Yes, and I think there might be another 1-2 bugs in unused cryptographic code. I did a quick check of closed tickets and I couldn't find them.

Maybe @dconnolly can help us find the buggy code we want to remove?

[teor2345]

This seems fine for now, let's work out what other code we need to delete in another PR.

[teor2345]

I think we can also remove the SpendingKey and derived keys from sprout, sapling, and orchard. As far as I can tell, they are only used in tests.

The SpendingKey structs for sapling, sprout and orchard are already marked as code that will only compile for tests so auditors should skip testing code.

Those aren't test-only markers for the struct. The Arbitrary derives let us generate random keys when we're running tests, and cfg_attr only changes the derive attribute, not the entire struct.

Do we want to remove the structs, implementations and tests alltogether ?

I think we might be able to delete all 6 shielded keys and addresses modules, because we don't implement a full node wallet yet.

[teor2345]

@Mergifyio update

[mergify]

update

✅ Branch has been successfully updated

[arya2]

https://github.com/ZcashFoundation/zebra/actions/runs/3316596195/jobs/5478977357#step:6:106:

Waiting for SSH key to propagate.
Unable to find image 'us-docker.pkg.dev/zealous-zebra/zebra/zebrad-test:sha-5ee64b7' locally
docker: Error response from daemon: Get "https://us-docker.pkg.dev/v2/zealous-zebra/zebra/zebrad-test/manifests/sha256:259f54c94df4e042ddeeddfa91b90634d08ed99a5e29a6588a62472130c9477b": dial tcp 173.194.193.82:443: i/o timeout.

Same problem here https://github.com/ZcashFoundation/zebra/actions/runs/3316596195/jobs/5478977357:

Waiting for SSH key to propagate.
Unable to find image 'us-docker.pkg.dev/zealous-zebra/zebra/zebrad-test:sha-5ee64b7' locally
docker: Error response from daemon: Get "https://us-docker.pkg.dev/v2/zealous-zebra/zebra/zebrad-test/manifests/sha256:259f54c94df4e042ddeeddfa91b90634d08ed99a5e29a6588a62472130c9477b": dial tcp 173.194.193.82:443: i/o timeout.

[teor2345]

We also want to remove all the code that is impacted by these known bugs:

* #4691

* the key parsing panics we found in PR #4732

* anything else?

Are any of these issues happening because of cryptographic implementations in zebra-chain that are unused?

Yes, and I think there might be another 1-2 bugs in unused cryptographic code. I did a quick check of closed tickets and I couldn't find them.

Maybe @dconnolly can help us find the buggy code we want to remove?

Let's continue this conversation on PR #5476?