Serialization tweaks #209
Serialization tweaks #209
Conversation
bazzilic
commented
Aug 28, 2023
bazzilic
commented
- Added serde serialization for all relevant structs
- Moved both serde and borsh serialization behind respective feature flags
taiga_halo2/src/transaction.rs
Outdated
| #[test] | ||
| fn test_halo2_transaction() { | ||
| fn test_halo2_transaction_borsh_serialize() { |
There was a problem hiding this comment.
We probably could separate the transaction test and the borsh test as we've added the feature flag?
There was a problem hiding this comment.
Ah, I didn't realize this was a combined test. Yeah, I can separate them. CI runs tests with --all-features anyway, so all feature tests are run too. But yeah, let me push an updated test.
There was a problem hiding this comment.
Updated the tests.
There was a problem hiding this comment.
Is it worth using the exported TX from test_halo2_transaction in test_halo2_transaction_borsh_serialize to avoid duplicated TX construction since it takes a long time to build a TX?
There was a problem hiding this comment.
I don't know how to pass data between tests though.
There was a problem hiding this comment.
Actually, fixed that
bazzilic
force-pushed
the
vasily/serde
branch
from
2828dc7
to
d579d03
Compare
bazzilic
force-pushed
the
vasily/serde
branch
from
d579d03
to
ee6a310
Compare
|
Changes that are left in this branch after it was rebased onto |
| halo2_gadgets = { git = "https://github.com/heliaxdev/halo2", branch = "taiga" } | ||
| halo2_proofs = { git = "https://github.com/heliaxdev/halo2", branch = "taiga" } | ||
| pasta_curves = { git = "https://github.com/heliaxdev/pasta_curves", branch = "taiga" } |
There was a problem hiding this comment.
Could we remove these patches as we've specified them in dependencies?
There was a problem hiding this comment.
No, unfortunately we can't. Unlike what we indicate in out dependencies, these patches also affect the whole dependency tree.
