sdk%refac: consolidate exports, types, add utf8 serde codec to process domain names, preserve unknown values, drop legacy macro#11
Merged
Conversation
📝 WalkthroughWalkthroughThis PR consolidates Dash SDK primitive types by relocating crypto byte wrappers into ChangesSDK primitive consolidation and crate API reshaping
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Note This pull request has no conflicts! 🎊 🎉 🎊 |
5 tasks
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pkgs/primitives/src/payload/proregtx.rs (1)
211-223:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate legacy
net_infoemptiness inProRegTx::check.
NetInfoType::Legacycurrently bypasses emptiness checks, so invalid v1/v2 service endpoints can pass validation.Suggested fix
- if let NetInfoType::Extended(ref ext) = self.net_info { - if ext.is_empty() { - return Some(ProTxInvalid::NetInfoEmpty); - } - if let Some(e) = check_sptx_netinfo(&ext.entries, self.mn_type, self.version >= PROTX_VERSION_EXT_ADDR) { - return Some(e); - } - } + match &self.net_info { + NetInfoType::Extended(ext) => { + if ext.is_empty() { + return Some(ProTxInvalid::NetInfoEmpty); + } + if let Some(e) = + check_sptx_netinfo(&ext.entries, self.mn_type, self.version >= PROTX_VERSION_EXT_ADDR) + { + return Some(e); + } + } + NetInfoType::Legacy(mn) => { + if mn.is_empty() { + return Some(ProTxInvalid::NetInfoEmpty); + } + } + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkgs/primitives/src/payload/proregtx.rs` around lines 211 - 223, The ProRegTx::check method validates the Extended variant of NetInfoType for emptiness but does not perform the same validation for the Legacy variant, allowing invalid legacy service endpoints to pass. Add an emptiness check for NetInfoType::Legacy (matching the pattern used for NetInfoType::Extended) that returns ProTxInvalid::NetInfoEmpty if the legacy entries collection is empty, ensuring both legacy and extended net_info types are validated consistently.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkgs/num/src/hash.rs`:
- Around line 16-24: The ParseHexError enum is missing the Hash derive trait in
its derive macro. Add Hash to the derive attribute on the ParseHexError enum
definition to comply with the project coding guidelines that require deriving
Clone, Debug, PartialEq, Eq, and Hash together on enums.
In `@pkgs/primitives/src/payload/proregtx.rs`:
- Around line 155-163: The encoding logic for net_info in the version check
branches fails silently when the version and net_info variant disagree (e.g.,
version >= 3 with Legacy, or version < 3 with Extended), producing corrupted
payloads instead of failing fast. Replace the if-let and else-if chain with a
match statement on net_info that validates the version constraint for each
variant, panicking or returning an error when NetInfoType::Extended is
encountered with version < 3 or NetInfoType::Legacy is encountered with version
>= 3, ensuring all cases are explicitly handled and mismatches are caught
immediately.
In `@pkgs/primitives/src/payload/proupservtx.rs`:
- Around line 109-117: The ProUpServTx encoding logic silently skips net_info
encoding when there is a mismatch between the version field and the NetInfoType
variant, which can result in malformed payloads. In the encode method, when
version >= 3, ensure the net_info is an Extended variant, and when version < 3,
ensure the net_info is a Legacy variant. Rather than using if let which silently
skips on mismatch, use pattern matching that panics or returns an error to
enforce the invariant that version and net_info variant must match. This
prevents silently serializing incomplete or malformed data.
In `@pkgs/primitives/src/types/netinfo.rs`:
- Around line 290-292: The is_empty method in ExtendedNetInfo is incomplete and
does not accurately reflect whether the struct contains any address data.
Currently it only checks if self.entries (purpose groups) is empty, but
according to the documented contract it must also verify that no actual address
entries exist. Update the is_empty method in ExtendedNetInfo to check all
relevant fields that store address information, ensuring it returns true only
when there are genuinely no addresses carried by the struct, not just when no
purpose groups exist.
- Around line 212-221: The match statement handling entry_type in the decode
function currently returns NetInfoEntry::Invalid for unknown entry types (the
default case at line 220) without consuming any payload bytes, which causes the
decode stream to become misaligned for subsequent entries. Replace the default
case that returns NetInfoEntry::Invalid with a proper error return that fails
fast (using appropriate error handling mechanism for your codec), ensuring that
unknown entry types are treated as decoding errors rather than silently accepted
values. This will prevent the data stream from being desynchronized when
unexpected entry types are encountered.
---
Outside diff comments:
In `@pkgs/primitives/src/payload/proregtx.rs`:
- Around line 211-223: The ProRegTx::check method validates the Extended variant
of NetInfoType for emptiness but does not perform the same validation for the
Legacy variant, allowing invalid legacy service endpoints to pass. Add an
emptiness check for NetInfoType::Legacy (matching the pattern used for
NetInfoType::Extended) that returns ProTxInvalid::NetInfoEmpty if the legacy
entries collection is empty, ensuring both legacy and extended net_info types
are validated consistently.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 4e8d54d8-e69f-4d1a-b037-d663923a497c
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (64)
contrib/semgrep/types.ymlpkgs/dev/src/lambda.rspkgs/num/src/error.rspkgs/num/src/hash.rspkgs/num/src/lib.rspkgs/p2p_core/src/lib.rspkgs/p2p_core/src/msg/addr.rspkgs/p2p_core/src/msg/cfcheckpt.rspkgs/p2p_core/src/msg/cfheaders.rspkgs/p2p_core/src/msg/cfilter.rspkgs/p2p_core/src/msg/compact_filters.rspkgs/p2p_core/src/msg/headers.rspkgs/p2p_core/src/msg/headers2.rspkgs/p2p_core/src/msg/inv.rspkgs/p2p_core/src/msg/mnlistdiff.rspkgs/p2p_core/src/msg/mod.rspkgs/p2p_core/src/msg/version.rspkgs/p2p_core/src/primitives/filter_type.rspkgs/p2p_core/src/primitives/mn_list.rspkgs/p2p_core/src/primitives/mod.rspkgs/p2p_core/src/primitives/net_addr.rspkgs/p2p_core/src/primitives/short_id.rspkgs/p2p_core/src/v2/mod.rspkgs/p2p_core/tests/addrv2.rspkgs/params/src/types.rspkgs/pkc/Cargo.tomlpkgs/pkc/src/bls_chia/pk.rspkgs/pkc/src/bls_chia/sig.rspkgs/pkc/src/bls_ietf/pk.rspkgs/pkc/src/bls_ietf/sig.rspkgs/pkc/src/k256/pk.rspkgs/pkc/src/k256/sig.rspkgs/pkc/src/lib.rspkgs/primitives/Cargo.tomlpkgs/primitives/src/block.rspkgs/primitives/src/block_header.rspkgs/primitives/src/gov.rspkgs/primitives/src/lib.rspkgs/primitives/src/outpoint.rspkgs/primitives/src/payload/assetlock.rspkgs/primitives/src/payload/assetunlock.rspkgs/primitives/src/payload/cbtx.rspkgs/primitives/src/payload/mnhftx.rspkgs/primitives/src/payload/mod.rspkgs/primitives/src/payload/proregtx.rspkgs/primitives/src/payload/proupregtx.rspkgs/primitives/src/payload/prouprevtx.rspkgs/primitives/src/payload/proupservtx.rspkgs/primitives/src/payload/quorum.rspkgs/primitives/src/script.rspkgs/primitives/src/support.rspkgs/primitives/src/transaction.rspkgs/primitives/src/tx_in.rspkgs/primitives/src/tx_out.rspkgs/primitives/src/tx_types.rspkgs/primitives/src/types/addrv1.rspkgs/primitives/src/types/addrv2.rspkgs/primitives/src/types/mod.rspkgs/primitives/src/types/netinfo.rspkgs/primitives/src/validation.rspkgs/script/Cargo.tomlpkgs/script/src/lib.rspkgs/types/src/lib.rspkgs/types/src/serialize.rs
💤 Files with no reviewable changes (14)
- pkgs/num/src/error.rs
- pkgs/primitives/src/tx_in.rs
- pkgs/script/src/lib.rs
- pkgs/p2p_core/src/primitives/filter_type.rs
- pkgs/primitives/src/tx_types.rs
- pkgs/p2p_core/src/msg/cfcheckpt.rs
- pkgs/primitives/src/block_header.rs
- pkgs/p2p_core/src/msg/mnlistdiff.rs
- pkgs/types/src/lib.rs
- pkgs/p2p_core/src/msg/cfheaders.rs
- pkgs/p2p_core/src/msg/cfilter.rs
- pkgs/primitives/src/tx_out.rs
- pkgs/primitives/src/outpoint.rs
- pkgs/primitives/src/validation.rs
Comment on lines
+16
to
+24
| #[derive(Clone, Debug, PartialEq, Eq)] | ||
| pub enum ParseHexError { | ||
| /// The hex string has an odd number of characters. | ||
| OddLength, | ||
| /// The decoded byte count does not match the expected length. | ||
| InvalidLength { expected: usize, got: usize }, | ||
| /// A non-hex character was encountered. | ||
| InvalidChar(u8), | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Add Hash derive on ParseHexError to align with project policy.
This enum currently derives Clone, Debug, PartialEq, and Eq, but misses Hash.
Proposed patch
-#[derive(Clone, Debug, PartialEq, Eq)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
pub enum ParseHexError {As per coding guidelines: "Derive Clone, Debug, PartialEq, Eq, Hash eagerly."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| #[derive(Clone, Debug, PartialEq, Eq)] | |
| pub enum ParseHexError { | |
| /// The hex string has an odd number of characters. | |
| OddLength, | |
| /// The decoded byte count does not match the expected length. | |
| InvalidLength { expected: usize, got: usize }, | |
| /// A non-hex character was encountered. | |
| InvalidChar(u8), | |
| } | |
| #[derive(Clone, Debug, PartialEq, Eq, Hash)] | |
| pub enum ParseHexError { | |
| /// The hex string has an odd number of characters. | |
| OddLength, | |
| /// The decoded byte count does not match the expected length. | |
| InvalidLength { expected: usize, got: usize }, | |
| /// A non-hex character was encountered. | |
| InvalidChar(u8), | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkgs/num/src/hash.rs` around lines 16 - 24, The ParseHexError enum is missing
the Hash derive trait in its derive macro. Add Hash to the derive attribute on
the ParseHexError enum definition to comply with the project coding guidelines
that require deriving Clone, Debug, PartialEq, Eq, and Hash together on enums.
Source: Coding guidelines
Comment on lines
+212
to
+221
| let entry_type = u8::decode(data)?; | ||
| let entry = match entry_type { | ||
| 0x01 => NetInfoEntry::Service(ServiceV1::decode(data)?), | ||
| 0x02 => { | ||
| let name: Vec<u8> = Vec::decode(data)?; | ||
| let port = codec::read_u16_be(data)?; | ||
| NetInfoEntry::Domain { name, port } | ||
| } | ||
| _ => NetInfoEntry::Invalid, | ||
| }; |
There was a problem hiding this comment.
Fail fast on unknown entry_type to prevent decode stream desynchronization.
Line 220 currently accepts unknown entry types as Invalid without consuming any payload bytes. That can misalign parsing for subsequent entries and fields.
Proposed fix
let entry = match entry_type {
0x01 => NetInfoEntry::Service(ServiceV1::decode(data)?),
0x02 => {
let name: Vec<u8> = Vec::decode(data)?;
let port = codec::read_u16_be(data)?;
NetInfoEntry::Domain { name, port }
}
- _ => NetInfoEntry::Invalid,
+ _ => {
+ return Err(DecodeError::InvalidValue {
+ expected: 0x02,
+ actual: u64::from(entry_type),
+ })
+ }
};🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkgs/primitives/src/types/netinfo.rs` around lines 212 - 221, The match
statement handling entry_type in the decode function currently returns
NetInfoEntry::Invalid for unknown entry types (the default case at line 220)
without consuming any payload bytes, which causes the decode stream to become
misaligned for subsequent entries. Replace the default case that returns
NetInfoEntry::Invalid with a proper error return that fails fast (using
appropriate error handling mechanism for your codec), ensuring that unknown
entry types are treated as decoding errors rather than silently accepted values.
This will prevent the data stream from being desynchronized when unexpected
entry types are encountered.
Comment on lines
+290
to
+292
| fn is_empty(&self) -> bool { | ||
| self.entries.is_empty() | ||
| } |
There was a problem hiding this comment.
ExtendedNetInfo::is_empty does not match its documented contract.
Line 291 only checks whether purpose groups exist, not whether any address entries exist. This can return false even when no addresses are carried.
Proposed fix
fn is_empty(&self) -> bool {
- self.entries.is_empty()
+ self.entries.iter().all(|(_, group)| {
+ group.iter().all(|entry| matches!(entry, NetInfoEntry::Invalid))
+ })
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| fn is_empty(&self) -> bool { | |
| self.entries.is_empty() | |
| } | |
| fn is_empty(&self) -> bool { | |
| self.entries.iter().all(|(_, group)| { | |
| group.iter().all(|entry| matches!(entry, NetInfoEntry::Invalid)) | |
| }) | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkgs/primitives/src/types/netinfo.rs` around lines 290 - 292, The is_empty
method in ExtendedNetInfo is incomplete and does not accurately reflect whether
the struct contains any address data. Currently it only checks if self.entries
(purpose groups) is empty, but according to the documented contract it must also
verify that no actual address entries exist. Update the is_empty method in
ExtendedNetInfo to check all relevant fields that store address information,
ensuring it returns true only when there are genuinely no addresses carried by
the struct, not just when no purpose groups exist.
kwvg
changed the title
utf8 serde codec to process domain names, preserve unknown types in GovObjectType utf8 serde codec to process domain names, preserve unknown types, drop legacy macro
Collaborator
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contrib/codeql/decl.ql`:
- Around line 88-99: The new branch checking for Unknown variant enforcement at
line 88 does not apply the same filtering conditions as other policy checks in
the query. Add guard conditions to ensure this branch only applies to evaluated
crates, serde internal types, and encodable types by wrapping the entire
exists(Enum e | bareUnknownVariant(e, item) and...) block with conditions
checking isEvaluatedCrate, isSerdeInternalType, and isNotEncodable, similar to
how other branches in the query filter their scope. This prevents false
positives outside the intended lint targets.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: b41b857b-e8b3-445e-bcec-415611dd6c51
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (66)
contrib/codeql/decl.qlcontrib/semgrep/types.ymlpkgs/dev/src/lambda.rspkgs/num/src/error.rspkgs/num/src/hash.rspkgs/num/src/lib.rspkgs/num/src/util.rspkgs/p2p_core/src/lib.rspkgs/p2p_core/src/msg/addr.rspkgs/p2p_core/src/msg/cfcheckpt.rspkgs/p2p_core/src/msg/cfheaders.rspkgs/p2p_core/src/msg/cfilter.rspkgs/p2p_core/src/msg/compact_filters.rspkgs/p2p_core/src/msg/headers.rspkgs/p2p_core/src/msg/headers2.rspkgs/p2p_core/src/msg/inv.rspkgs/p2p_core/src/msg/mnlistdiff.rspkgs/p2p_core/src/msg/mod.rspkgs/p2p_core/src/msg/version.rspkgs/p2p_core/src/primitives/filter_type.rspkgs/p2p_core/src/primitives/mn_list.rspkgs/p2p_core/src/primitives/mod.rspkgs/p2p_core/src/primitives/net_addr.rspkgs/p2p_core/src/primitives/short_id.rspkgs/p2p_core/src/v2/mod.rspkgs/p2p_core/tests/addrv2.rspkgs/params/src/types.rspkgs/pkc/Cargo.tomlpkgs/pkc/src/bls_chia/pk.rspkgs/pkc/src/bls_chia/sig.rspkgs/pkc/src/bls_ietf/pk.rspkgs/pkc/src/bls_ietf/sig.rspkgs/pkc/src/k256/pk.rspkgs/pkc/src/k256/sig.rspkgs/pkc/src/lib.rspkgs/primitives/Cargo.tomlpkgs/primitives/src/block.rspkgs/primitives/src/block_header.rspkgs/primitives/src/gov.rspkgs/primitives/src/lib.rspkgs/primitives/src/outpoint.rspkgs/primitives/src/payload/assetlock.rspkgs/primitives/src/payload/assetunlock.rspkgs/primitives/src/payload/cbtx.rspkgs/primitives/src/payload/mnhftx.rspkgs/primitives/src/payload/mod.rspkgs/primitives/src/payload/proregtx.rspkgs/primitives/src/payload/proupregtx.rspkgs/primitives/src/payload/prouprevtx.rspkgs/primitives/src/payload/proupservtx.rspkgs/primitives/src/payload/quorum.rspkgs/primitives/src/script.rspkgs/primitives/src/support.rspkgs/primitives/src/transaction.rspkgs/primitives/src/tx_in.rspkgs/primitives/src/tx_out.rspkgs/primitives/src/tx_types.rspkgs/primitives/src/types/addrv1.rspkgs/primitives/src/types/addrv2.rspkgs/primitives/src/types/mod.rspkgs/primitives/src/types/netinfo.rspkgs/primitives/src/validation.rspkgs/script/Cargo.tomlpkgs/script/src/lib.rspkgs/types/src/lib.rspkgs/types/src/serialize.rs
💤 Files with no reviewable changes (15)
- pkgs/primitives/src/tx_out.rs
- pkgs/num/src/error.rs
- pkgs/primitives/src/outpoint.rs
- pkgs/primitives/src/tx_types.rs
- pkgs/p2p_core/src/msg/cfilter.rs
- pkgs/types/src/lib.rs
- pkgs/primitives/src/tx_in.rs
- pkgs/p2p_core/src/msg/cfcheckpt.rs
- pkgs/num/src/util.rs
- pkgs/p2p_core/src/primitives/filter_type.rs
- pkgs/p2p_core/src/msg/mnlistdiff.rs
- pkgs/p2p_core/src/msg/cfheaders.rs
- pkgs/primitives/src/block_header.rs
- pkgs/primitives/src/validation.rs
- pkgs/p2p_core/src/primitives/net_addr.rs
✅ Files skipped from review due to trivial changes (13)
- pkgs/p2p_core/src/primitives/short_id.rs
- pkgs/p2p_core/src/msg/headers2.rs
- pkgs/p2p_core/src/msg/inv.rs
- pkgs/primitives/src/payload/assetunlock.rs
- pkgs/primitives/src/types/mod.rs
- pkgs/pkc/Cargo.toml
- pkgs/pkc/src/lib.rs
- pkgs/p2p_core/tests/addrv2.rs
- pkgs/primitives/src/payload/cbtx.rs
- pkgs/primitives/src/payload/assetlock.rs
- pkgs/p2p_core/src/msg/headers.rs
- pkgs/p2p_core/src/v2/mod.rs
- pkgs/primitives/src/payload/proupregtx.rs
🚧 Files skipped from review as they are similar to previous changes (29)
- pkgs/primitives/src/payload/mnhftx.rs
- pkgs/dev/src/lambda.rs
- pkgs/params/src/types.rs
- pkgs/types/src/serialize.rs
- contrib/semgrep/types.yml
- pkgs/primitives/src/types/addrv2.rs
- pkgs/p2p_core/src/lib.rs
- pkgs/primitives/Cargo.toml
- pkgs/primitives/src/payload/quorum.rs
- pkgs/pkc/src/k256/sig.rs
- pkgs/pkc/src/bls_ietf/pk.rs
- pkgs/primitives/src/payload/prouprevtx.rs
- pkgs/pkc/src/bls_ietf/sig.rs
- pkgs/script/Cargo.toml
- pkgs/pkc/src/bls_chia/sig.rs
- pkgs/pkc/src/k256/pk.rs
- pkgs/primitives/src/script.rs
- pkgs/num/src/hash.rs
- pkgs/p2p_core/src/primitives/mn_list.rs
- pkgs/p2p_core/src/msg/mod.rs
- pkgs/pkc/src/bls_chia/pk.rs
- pkgs/primitives/src/transaction.rs
- pkgs/num/src/lib.rs
- pkgs/primitives/src/lib.rs
- pkgs/p2p_core/src/msg/compact_filters.rs
- pkgs/primitives/src/block.rs
- pkgs/primitives/src/support.rs
- pkgs/primitives/src/payload/mod.rs
- pkgs/primitives/src/payload/proregtx.rs
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@contrib/git_filter.py`:
- Around line 219-223: The exception handler in the try-except block around
main() is catching the overly broad Exception class, which masks unexpected
defects. Replace the Exception catch with a tuple of the specific exception
types that main() can raise: RuntimeError (from _git_ok() when git commands
fail), FileNotFoundError (from root_dir() if the workspace root is not found),
and OSError and its subclasses (from tempfile.mkdtemp()). This allows unexpected
exceptions to propagate while still handling all expected operational failures.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 2aef0dee-0bd2-486e-abbe-66d2ac47b704
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (61)
contrib/codeql/decl.qlcontrib/common.pycontrib/git_filter.pycontrib/lint_all.pycontrib/semgrep/types.ymlpkgs/num/src/error.rspkgs/num/src/hash.rspkgs/num/src/lib.rspkgs/num/src/util.rspkgs/p2p_core/src/lib.rspkgs/p2p_core/src/msg/addr.rspkgs/p2p_core/src/msg/cfcheckpt.rspkgs/p2p_core/src/msg/cfheaders.rspkgs/p2p_core/src/msg/cfilter.rspkgs/p2p_core/src/msg/compact_filters.rspkgs/p2p_core/src/msg/mnlistdiff.rspkgs/p2p_core/src/msg/mod.rspkgs/p2p_core/src/msg/version.rspkgs/p2p_core/src/primitives/filter_type.rspkgs/p2p_core/src/primitives/mn_list.rspkgs/p2p_core/src/primitives/mod.rspkgs/p2p_core/src/primitives/net_addr.rspkgs/p2p_core/tests/addrv2.rspkgs/pkc/Cargo.tomlpkgs/pkc/src/bls_chia/pk.rspkgs/pkc/src/bls_chia/sig.rspkgs/pkc/src/bls_ietf/pk.rspkgs/pkc/src/bls_ietf/sig.rspkgs/pkc/src/k256/pk.rspkgs/pkc/src/k256/sig.rspkgs/pkc/src/lib.rspkgs/primitives/Cargo.tomlpkgs/primitives/src/block.rspkgs/primitives/src/block_header.rspkgs/primitives/src/gov.rspkgs/primitives/src/lib.rspkgs/primitives/src/outpoint.rspkgs/primitives/src/payload/assetlock.rspkgs/primitives/src/payload/assetunlock.rspkgs/primitives/src/payload/cbtx.rspkgs/primitives/src/payload/mnhftx.rspkgs/primitives/src/payload/mod.rspkgs/primitives/src/payload/proregtx.rspkgs/primitives/src/payload/proupregtx.rspkgs/primitives/src/payload/prouprevtx.rspkgs/primitives/src/payload/proupservtx.rspkgs/primitives/src/payload/quorum.rspkgs/primitives/src/script.rspkgs/primitives/src/support.rspkgs/primitives/src/transaction.rspkgs/primitives/src/tx_in.rspkgs/primitives/src/tx_out.rspkgs/primitives/src/types/addrv1.rspkgs/primitives/src/types/addrv2.rspkgs/primitives/src/types/mod.rspkgs/primitives/src/types/netinfo.rspkgs/primitives/src/validation.rspkgs/script/Cargo.tomlpkgs/script/src/lib.rspkgs/types/src/lib.rspkgs/types/src/serialize.rs
💤 Files with no reviewable changes (14)
- pkgs/primitives/src/block_header.rs
- pkgs/p2p_core/src/msg/cfheaders.rs
- pkgs/p2p_core/src/primitives/filter_type.rs
- pkgs/num/src/util.rs
- pkgs/p2p_core/src/msg/cfcheckpt.rs
- pkgs/primitives/src/tx_in.rs
- pkgs/primitives/src/outpoint.rs
- pkgs/types/src/lib.rs
- pkgs/p2p_core/src/primitives/net_addr.rs
- pkgs/p2p_core/src/msg/cfilter.rs
- pkgs/p2p_core/src/msg/mnlistdiff.rs
- pkgs/primitives/src/validation.rs
- pkgs/primitives/src/tx_out.rs
- pkgs/num/src/error.rs
✅ Files skipped from review due to trivial changes (8)
- pkgs/primitives/src/payload/cbtx.rs
- pkgs/pkc/Cargo.toml
- pkgs/primitives/src/payload/assetunlock.rs
- pkgs/primitives/src/payload/proupregtx.rs
- pkgs/primitives/src/types/mod.rs
- pkgs/types/src/serialize.rs
- pkgs/primitives/src/payload/prouprevtx.rs
- pkgs/p2p_core/tests/addrv2.rs
🚧 Files skipped from review as they are similar to previous changes (34)
- contrib/semgrep/types.yml
- pkgs/primitives/src/types/addrv1.rs
- contrib/codeql/decl.ql
- pkgs/primitives/src/payload/assetlock.rs
- pkgs/primitives/src/payload/mnhftx.rs
- pkgs/primitives/src/gov.rs
- pkgs/pkc/src/bls_ietf/sig.rs
- pkgs/pkc/src/bls_chia/pk.rs
- pkgs/pkc/src/lib.rs
- pkgs/pkc/src/bls_ietf/pk.rs
- pkgs/pkc/src/k256/sig.rs
- pkgs/pkc/src/k256/pk.rs
- pkgs/p2p_core/src/primitives/mod.rs
- pkgs/num/src/hash.rs
- pkgs/primitives/src/script.rs
- pkgs/primitives/src/types/addrv2.rs
- pkgs/primitives/src/payload/quorum.rs
- pkgs/script/Cargo.toml
- pkgs/num/src/lib.rs
- pkgs/p2p_core/src/msg/version.rs
- pkgs/pkc/src/bls_chia/sig.rs
- pkgs/p2p_core/src/primitives/mn_list.rs
- pkgs/primitives/Cargo.toml
- pkgs/p2p_core/src/msg/mod.rs
- pkgs/primitives/src/payload/proregtx.rs
- pkgs/p2p_core/src/lib.rs
- pkgs/primitives/src/payload/proupservtx.rs
- pkgs/p2p_core/src/msg/addr.rs
- pkgs/p2p_core/src/msg/compact_filters.rs
- pkgs/primitives/src/transaction.rs
- pkgs/primitives/src/types/netinfo.rs
- pkgs/primitives/src/block.rs
- pkgs/primitives/src/payload/mod.rs
- pkgs/script/src/lib.rs
kwvg
changed the title
utf8 serde codec to process domain names, preserve unknown types, drop legacy macroutf8 serde codec to process domain names, preserve unknown values, drop legacy macro
This was referenced Jun 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
Our codec work in base-sdk#4 reduced a lot of boilerplate required to define types, this pull request does a lot of code reorganisation that's now possible due to definitions being overall shorter. Plus we want to start stabilising names as upcoming pull requests may use them as fixed identifiers.
Additional Information
GovObjectType::UnknownandScriptKind::Unknownnow preserve the raw integer value instead of discarding it, ensuring better forward compatibility.A UTF-8 serde helper has been added to correctly render
Domain::nameas a string rather than raw bytes to ensure they're rendered correctly in the parser demo introduced in base-sdk#10Breaking Changes
All internal modules in
primitivesandp2p_corehave been made private. Public items are re-exported directly from crate roots, souse dash_primitives::BlockHeadercontinues to work whileuse dash_primitives::block_header::BlockHeaderdoes not.CServicehas been renamed toServiceV1.NetAddrhas been renamed toVersionAddr.Key container newtypes have moved from
dash_typestodash_pkc.The helper macro
dash_num::util::make_hash256!has been dropped.How Has This Been Tested?
Checklist