From 277223da6a70149a27713b9de169c4f498a2bf83 Mon Sep 17 00:00:00 2001
From: Martin Habovstiak <martin.habovstiak@gmail.com>
Date: Thu, 20 Feb 2025 15:35:17 +0100
Subject: [PATCH] Make `Script` and `ScriptBuf` obey sanity rules

The newtype sanity rules (a name I came up with):
* Newtypes should have at most one constructor that directly references
  the inner field.
* Newtypes should have at most three accessor methods that directly
  reference the ineer field: one for owned access, the second for
  borrowed and the third for mutably borrowed.
* All other methods should use the methods above to perform operations
  on the newtype and not directly access the fields.

This commit makes `Script` and `ScriptBuf` obey these except for
`reserve` and `reserve_exact` since we don't have `as_mut_vec` method.
As a side effect it also adds `const` to `ScriptBuf::from_bytes`.
---
 primitives/src/script/borrowed.rs | 14 +++++++-------
 primitives/src/script/mod.rs      | 12 ++++++------
 primitives/src/script/owned.rs    | 16 ++++++++--------
 3 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/primitives/src/script/borrowed.rs b/primitives/src/script/borrowed.rs
index 6e68a6ef6..b16d1c613 100644
--- a/primitives/src/script/borrowed.rs
+++ b/primitives/src/script/borrowed.rs
@@ -54,7 +54,7 @@ use crate::prelude::{Box, ToOwned, Vec};
 ///
 #[derive(PartialOrd, Ord, PartialEq, Eq, Hash)]
 #[repr(transparent)]
-pub struct Script(pub(in crate::script) [u8]);
+pub struct Script([u8]);
 
 impl Default for &Script {
     #[inline]
@@ -64,7 +64,7 @@ impl Default for &Script {
 impl ToOwned for Script {
     type Owned = ScriptBuf;
 
-    fn to_owned(&self) -> Self::Owned { ScriptBuf(self.0.to_owned()) }
+    fn to_owned(&self) -> Self::Owned { ScriptBuf::from_bytes(self.to_vec()) }
 }
 
 impl Script {
@@ -104,7 +104,7 @@ impl Script {
 
     /// Returns a copy of the script data.
     #[inline]
-    pub fn to_vec(&self) -> Vec<u8> { self.0.to_owned() }
+    pub fn to_vec(&self) -> Vec<u8> { self.as_bytes().to_owned() }
 
     /// Returns a copy of the script data.
     #[inline]
@@ -113,11 +113,11 @@ impl Script {
 
     /// Returns the length in bytes of the script.
     #[inline]
-    pub fn len(&self) -> usize { self.0.len() }
+    pub fn len(&self) -> usize { self.as_bytes().len() }
 
     /// Returns whether the script is the empty script.
     #[inline]
-    pub fn is_empty(&self) -> bool { self.0.is_empty() }
+    pub fn is_empty(&self) -> bool { self.as_bytes().is_empty() }
 
     /// Converts a [`Box<Script>`](Box) into a [`ScriptBuf`] without copying or allocating.
     #[must_use]
@@ -128,7 +128,7 @@ impl Script {
         // Casting a transparent struct wrapping a slice to the slice pointer is sound (same
         // layout).
         let inner = unsafe { Box::from_raw(rw) };
-        ScriptBuf(Vec::from(inner))
+        ScriptBuf::from_bytes(Vec::from(inner))
     }
 }
 
@@ -141,7 +141,7 @@ macro_rules! delegate_index {
 
                 #[inline]
                 fn index(&self, index: $type) -> &Self::Output {
-                    Self::from_bytes(&self.0[index])
+                    Self::from_bytes(&self.as_bytes()[index])
                 }
             }
         )*
diff --git a/primitives/src/script/mod.rs b/primitives/src/script/mod.rs
index dda224022..d628de796 100644
--- a/primitives/src/script/mod.rs
+++ b/primitives/src/script/mod.rs
@@ -236,7 +236,7 @@ impl<'a> From<&'a Script> for Cow<'a, Script> {
 #[cfg(target_has_atomic = "ptr")]
 impl<'a> From<&'a Script> for Arc<Script> {
     fn from(value: &'a Script) -> Self {
-        let rw: *const [u8] = Arc::into_raw(Arc::from(&value.0));
+        let rw: *const [u8] = Arc::into_raw(Arc::from(value.as_bytes()));
         // SAFETY: copied from `std`
         // The pointer was just created from an Arc without deallocating
         // Casting a slice to a transparent struct wrapping that slice is sound (same
@@ -247,7 +247,7 @@ impl<'a> From<&'a Script> for Arc<Script> {
 
 impl<'a> From<&'a Script> for Rc<Script> {
     fn from(value: &'a Script) -> Self {
-        let rw: *const [u8] = Rc::into_raw(Rc::from(&value.0));
+        let rw: *const [u8] = Rc::into_raw(Rc::from(value.as_bytes()));
         // SAFETY: copied from `std`
         // The pointer was just created from an Rc without deallocating
         // Casting a slice to a transparent struct wrapping that slice is sound (same
@@ -257,11 +257,11 @@ impl<'a> From<&'a Script> for Rc<Script> {
 }
 
 impl From<Vec<u8>> for ScriptBuf {
-    fn from(v: Vec<u8>) -> Self { ScriptBuf(v) }
+    fn from(v: Vec<u8>) -> Self { ScriptBuf::from_bytes(v) }
 }
 
 impl From<ScriptBuf> for Vec<u8> {
-    fn from(v: ScriptBuf) -> Self { v.0 }
+    fn from(v: ScriptBuf) -> Self { v.into_bytes() }
 }
 
 impl AsRef<Script> for Script {
@@ -418,11 +418,11 @@ impl fmt::UpperHex for ScriptBuf {
 impl Deref for ScriptBuf {
     type Target = Script;
 
-    fn deref(&self) -> &Self::Target { Script::from_bytes(&self.0) }
+    fn deref(&self) -> &Self::Target { self.as_script() }
 }
 
 impl DerefMut for ScriptBuf {
-    fn deref_mut(&mut self) -> &mut Self::Target { Script::from_bytes_mut(&mut self.0) }
+    fn deref_mut(&mut self) -> &mut Self::Target { self.as_mut_script() }
 }
 
 impl Borrow<Script> for ScriptBuf {
diff --git a/primitives/src/script/owned.rs b/primitives/src/script/owned.rs
index 29950cbfb..bf9e8bc71 100644
--- a/primitives/src/script/owned.rs
+++ b/primitives/src/script/owned.rs
@@ -19,17 +19,17 @@ use crate::prelude::{Box, Vec};
 ///
 /// [deref coercions]: https://doc.rust-lang.org/std/ops/trait.Deref.html#more-on-deref-coercion
 #[derive(Default, Clone, PartialOrd, Ord, PartialEq, Eq, Hash)]
-pub struct ScriptBuf(pub(in crate::script) Vec<u8>);
+pub struct ScriptBuf(Vec<u8>);
 
 impl ScriptBuf {
     /// Constructs a new empty script.
     #[inline]
-    pub const fn new() -> Self { ScriptBuf(Vec::new()) }
+    pub const fn new() -> Self { ScriptBuf::from_bytes(Vec::new()) }
 
     /// Converts byte vector into script.
     ///
     /// This method doesn't (re)allocate.
-    pub fn from_bytes(bytes: Vec<u8>) -> Self { ScriptBuf(bytes) }
+    pub const fn from_bytes(bytes: Vec<u8>) -> Self { ScriptBuf(bytes) }
 
     /// Returns a reference to unsized script.
     pub fn as_script(&self) -> &Script { Script::from_bytes(&self.0) }
@@ -52,12 +52,12 @@ impl ScriptBuf {
     #[inline]
     pub fn into_boxed_script(self) -> Box<Script> {
         // Copied from PathBuf::into_boxed_path
-        let rw = Box::into_raw(self.0.into_boxed_slice()) as *mut Script;
+        let rw = Box::into_raw(self.into_bytes().into_boxed_slice()) as *mut Script;
         unsafe { Box::from_raw(rw) }
     }
 
     /// Constructs a new empty script with pre-allocated capacity.
-    pub fn with_capacity(capacity: usize) -> Self { ScriptBuf(Vec::with_capacity(capacity)) }
+    pub fn with_capacity(capacity: usize) -> Self { ScriptBuf::from_bytes(Vec::with_capacity(capacity)) }
 
     /// Pre-allocates at least `additional_len` bytes if needed.
     ///
@@ -91,7 +91,7 @@ impl ScriptBuf {
 impl<'a> Arbitrary<'a> for ScriptBuf {
     fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
         let v = Vec::<u8>::arbitrary(u)?;
-        Ok(ScriptBuf(v))
+        Ok(ScriptBuf::from_bytes(v))
     }
 }
 
@@ -103,7 +103,7 @@ mod tests {
     fn script_buf_from_bytes() {
         let bytes = vec![1, 2, 3];
         let script = ScriptBuf::from_bytes(bytes.clone());
-        assert_eq!(script.0, bytes);
+        assert_eq!(script.as_bytes(), bytes);
     }
 
     #[test]
@@ -120,7 +120,7 @@ mod tests {
         let mut script = ScriptBuf::from_bytes(bytes.clone());
         let script_mut_ref = script.as_mut_script();
         script_mut_ref.as_mut_bytes()[0] = 4;
-        assert_eq!(script.0, vec![4, 2, 3]);
+        assert_eq!(script.as_mut_bytes(), vec![4, 2, 3]);
     }
 
     #[test]