zingolabs / zingolib

An API and test-app that exposes zcash functionality for app consumption
MIT License
15 stars 23 forks source link

Small changes requiring serialization changes to implement #998

Open AloeareV opened 6 months ago

AloeareV commented 6 months ago
          I am confused about the presence of both `u64` and `u32` for..  ?similar? fields.

          Is the regression proptest data because of this variance?  Is it correct?

Originally posted by @zancas in https://github.com/zingolabs/zingolib/pull/992#pullrequestreview-2021431681

This needs a serialization version increment to align, (and handling for if we somehow read a None), which is fiddly and we likely don't have time for before zip317.

This would be a good place to keep track of anything else that necessitates a version bump, in case we can do multiple changes at once and make the versioning simpler.

AloeareV commented 6 months ago

If we add the ability to get a transaction record's fee, that could be worth saving as an optional value to avoid needing to re-fetch every time we reload the wallet.

Oscar-Pepper commented 6 months ago

If we add the ability to get a transaction record's fee, that could be worth saving as an optional value to avoid needing to re-fetch every time we reload the wallet.

it is actually only currently possible to calculate the fee from OutgoingTxData which isnt mandatory data. I propose we calculate the fee by summing each pools value balance (therefore fetching any Txs for the value of the transparent inputs) during sync and storing this in a fee field of transaction record in the wallet file, when we bump to a new wallet version.