-
Notifications
You must be signed in to change notification settings - Fork 0
feat: basic ferment integration #60
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
base: master
Are you sure you want to change the base?
Changes from all commits
76b2750
b070b55
7d12f2a
dc50bc0
be8759c
7dd0282
eb173d2
4128656
6f7c7af
d16e862
49dbc11
55476ce
cd13a23
6d62218
16ade0a
26f19af
94489ca
a28495f
fde599b
6623623
9ae30cd
73b5df6
c8b35fd
b17e4a4
0a7e342
e6027b9
7efb245
00ee68c
ead6df7
beef9fd
5df4261
de0019c
f9f71da
d72b6ef
80f14ca
59f10e3
0a29b27
ca6342a
11865a4
6bd8705
e1a4ebd
841e189
4fcb9bc
8f2f79c
8562906
872dc7c
d6b81c7
a385d58
1c80643
6351ba0
37ce04e
e9590c2
1459f41
6afc508
f748f6e
df893fb
c051fcc
3899a54
61d8ec5
f6c2837
534ba33
aa4e98a
9cd29dd
729d5e1
f5213fe
1aeeb4b
d7ef308
ff4a39c
eb511ff
7d23770
6b336b9
6dd3799
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,20 +32,21 @@ use crate::{Script, VarInt}; | |
/// [segwit upgrade]: <https://github.com/bitcoin/bips/blob/master/bip-0143.mediawiki> | ||
#[derive(Clone, Default, PartialEq, Eq, PartialOrd, Ord, Debug, Hash)] | ||
#[cfg_attr(feature = "bincode", derive(Encode, Decode))] | ||
#[cfg_attr(feature = "apple", ferment_macro::export)] | ||
pub struct Witness { | ||
/// Contains the witness `Vec<Vec<u8>>` serialization without the initial varint indicating the | ||
/// number of elements (which is stored in `witness_elements`). | ||
content: Vec<u8>, | ||
pub content: Vec<u8>, | ||
|
||
/// The number of elements in the witness. | ||
/// | ||
/// Stored separately (instead of as a VarInt in the initial part of content) so that methods | ||
/// like [`Witness::push`] don't have to shift the entire array. | ||
witness_elements: usize, | ||
pub witness_elements: usize, | ||
|
||
/// This is the valid index pointing to the beginning of the index area. This area is 4 * | ||
/// stack_size bytes at the end of the content vector which stores the indices of each item. | ||
indices_start: usize, | ||
pub indices_start: usize, | ||
Comment on lines
+35
to
+49
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💡 Verification agent 🧩 Analysis chainVerify serialization behavior with the ferment integration The export attribute and field visibility changes suggest that this struct will be serialized/deserialized by ferment when the "apple" feature is enabled. Ensure that the serialized representation correctly handles the complex internal structure of the Witness struct. 🏁 Script executed: #!/bin/bash
# Look for tests that verify the ferment serialization/deserialization of Witness
echo "Checking for tests related to ferment serialization of Witness"
rg -l "ferment.*Witness" --type rust
echo "Checking for ferment-related attributes in the codebase"
rg -A 2 -B 2 "#\[cfg_attr\(feature = \"apple\", ferment_macro::export\)\]" --type rust
echo "Checking for other structs with similar changes"
rg -l "pub content: Vec<u8>" --type rust Length of output: 15635 Action Required: Add explicit tests for ferment serialization of Witness After verifying the repository, we noticed that while multiple structs (e.g., those in Recommendations:
|
||
} | ||
|
||
/// An iterator returning individual witness elements. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Attribution addition for ferment integration matches other similar changes.
The addition of the
#[cfg_attr(feature = "apple", ferment_macro::export)]
attribute aligns with the overall PR objective of integrating with ferment when the "apple" feature flag is enabled.Looking at the previous review comment, there's a concern about testing the ferment serialization of the
Witness
struct. Let's verify if any tests have been added since that comment:🏁 Script executed:
Length of output: 529
Action Required: Add Tests for Ferment Serialization of the Witness Struct
The new attribute
#[cfg_attr(feature = "apple", ferment_macro::export)]
is in line with similar changes across the codebase, which is great. However, our current search did not uncover any tests verifying the ferment serialization of theWitness
struct. To ensure that this integration behaves as expected when the "apple" feature is enabled, please add tests covering this functionality or point to where such tests exist if they’ve been added elsewhere.dash/src/blockdata/witness.rs
(line 35)