qr: the medium never sees a partial word

`Z` compresses the whole file before cutting it up, so a single code is a slice of one
deflate stream and cannot be expanded on its own -- the stream is staged whole and
expanded afterwards. What can be fixed is what reaches the part while that happens.

**Every write is now a whole number of aligned words.** A short write makes the area
read the word back to merge with, and the expansion is the one place in a QR transfer
where reads and writes really are interleaved at full speed rather than a part at a
time -- everywhere else there are hundreds of milliseconds between accesses. The output
is sequential, so three bytes of carry is the whole fix: an odd tail waits for the front
of the next chunk to complete its word. The last word may be padded, because nothing
follows the image's end. A test walks every write the expansion made and checks both the
offset and the length.

Our own sender goes back to a multiple of twenty: five for base32's groups, four for the
device's words. This is not the refusal that was removed -- the device still accepts
whatever part size a sender chose, because another wallet's BBQr is not ours to dictate
and its driver merges an odd edge rather than refusing. It is only that when the sender
*is* ours there is no reason to make it merge anything, and then the medium is not read
even once during the whole transfer.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: MagicalTux

crates/catcard-upgrade/src/expand.rs

@@ -71,6 +71,15 @@ pub fn inflate<A: StagingArea>(
     // can see the other half-done.
     let mut read_at = 0u32;
     let mut write_at = 0u32;
+    // The tail of the last chunk, when it did not end on a word boundary.
+    //
+    // **The medium only ever sees whole aligned words.** A partial word would make the
+    // area read the word back to merge with, and this is the one place in the transfer
+    // where reads and writes really are interleaved at full speed rather than a part at
+    // a time. The output is sequential, so three bytes of carry is the whole fix: the
+    // odd tail waits for the front of the next chunk to complete its word.
+    let mut carry = [0u8; 4];
+    let mut carried = 0usize;
 
     let outcome = {
         let reader = Reader::new(chunk, |buf: &mut [u8]| {
@@ -86,13 +95,45 @@ pub fn inflate<A: StagingArea>(
         });
         let stream = Stream::new(window, max as u64, |data: &[u8]| {
             let mut area = cell.try_borrow_mut().map_err(|_| ZError::Io)?;
-            area.write(write_at, data).map_err(|_| ZError::Io)?;
-            write_at += data.len() as u32;
+            let mut data = data;
+
+            // Finish the word left over from last time before anything else.
+            if carried > 0 {
+                let take = data.len().min(4 - carried);
+                carry[carried..carried + take].copy_from_slice(&data[..take]);
+                carried += take;
+                data = &data[take..];
+                if carried < 4 {
+                    return Ok(());
+                }
+                area.write(write_at, &carry).map_err(|_| ZError::Io)?;
+                write_at += 4;
+                carried = 0;
+            }
+
+            let whole = data.len() & !3;
+            if whole > 0 {
+                area.write(write_at, &data[..whole])
+                    .map_err(|_| ZError::Io)?;
+                write_at += whole as u32;
+            }
+            carried = data.len() - whole;
+            carry[..carried].copy_from_slice(&data[whole..]);
             Ok(())
         });
         minizlib::inflate(reader, stream)
     };
 
+    // The last few bytes of the image, if it does not end on a word boundary. Nothing
+    // follows them, so the word they complete holds nothing that matters and the
+    // padding is past the image's length.
+    if outcome.is_ok() && carried > 0 {
+        carry[carried..].fill(0);
+        cell.borrow_mut()
+            .write(write_at, &carry)
+            .map_err(|_| Error::Storage)?;
+    }
+
     match outcome {
         Ok(n) => Ok(n as u32),
         Err(ZError::WindowTooSmall) => Err(Error::WindowTooSmall),

crates/catcard-upgrade/src/tests.rs

@@ -893,6 +893,39 @@ mod expanding {
         );
     }
 
+    /// **The medium never sees a partial word.** A short write would make the area read
+    /// the word back to merge with, and the expansion is the one place in a QR transfer
+    /// where reads and writes are interleaved at full speed rather than a part at a
+    /// time. Every write but the last is a whole number of aligned words; the last may
+    /// be padded, because nothing follows the image's end.
+    #[test]
+    fn every_write_is_whole_aligned_words() {
+        let image = firmwareish(200 * 1024 + 3);
+        let stream = deflate(&image, SENDER_WINDOW);
+        let mut mem = staged(&stream);
+        let mut window = vec![0u8; 8 * 1024];
+        let mut chunk = vec![0u8; 1024];
+        let n = expand::inflate(
+            &mut mem,
+            FROM,
+            stream.len() as u32,
+            image.len() as u32 + 4,
+            &mut window,
+            &mut chunk,
+        )
+        .expect("expands");
+        assert_eq!(n as usize, image.len());
+
+        for &(offset, len) in &mem.writes {
+            if offset == FROM {
+                continue; // the staging write this test did itself
+            }
+            assert_eq!(offset % 4, 0, "wrote {len} at unaligned {offset}");
+            assert_eq!(len % 4, 0, "wrote a partial word of {len} at {offset}");
+        }
+        assert_eq!(&mem.bytes[..image.len()], &image[..]);
+    }
+
     /// **Raw deflate carries no checksum**, so a damaged stream can expand to exactly
     /// the right length and simply be the wrong bytes -- there is nothing in the format
     /// to notice. That is not a gap to be closed here: the image's signature is checked

tools/catcard-image/src/qrpage.rs

@@ -31,15 +31,19 @@ const MAX_VERSION: u8 = 40;
 
 /// Bytes per part, unless the caller says otherwise.
 ///
-/// **A multiple of five**, which is base32's own constraint: eight characters carry five
-/// bytes, so a part that is not the last must be a whole number of groups or the parts
-/// after it do not decode on their own.
+/// **A multiple of twenty.** Five is base32's own constraint -- eight characters carry
+/// five bytes, so a part that is not the last must be a whole number of groups or the
+/// parts after it do not decode on their own. Four is the device's: parts land in PSRAM,
+/// which takes whole aligned words, and a part whose length is not a multiple of four
+/// puts every part after it at an unaligned offset.
 ///
-/// 1275 because the device's scanner buffer holds 2048 characters and 1275 bytes is
-/// 2048 of them with the header -- the largest part that fits. There is no alignment
-/// rule on top of that: parts land in PSRAM through a driver that paces them, and at one
-/// part every few hundred milliseconds the odd unaligned edge has all the time it needs.
-pub const DEFAULT_PART: usize = 1275;
+/// The device does **not** require this -- it cannot, since another wallet's BBQr parts
+/// are whatever size that wallet chose, and its driver merges an odd edge rather than
+/// refusing. But when the sender is ours there is no reason to make it: every part lands
+/// as whole words and the medium is never read during the transfer at all.
+///
+/// 1260 is the largest such size whose line still fits the device's scanner buffer.
+pub const DEFAULT_PART: usize = 1260;
 
 /// Compress the image if that makes fewer codes, and say which was used.
 ///
@@ -282,19 +286,27 @@ mod tests {
         assert_eq!(b64(b"foobar"), "Zm9vYmFy");
     }
 
-    /// The default is the largest part whose line still fits the device's scanner
-    /// buffer, and a whole number of base32 groups. Both fail quietly if they drift: an
-    /// over-long line is simply never read, and a part that is not a whole number of
-    /// groups puts every part after the first at the wrong offset.
+    /// The default is the largest part that satisfies both formats and still fits the
+    /// device's scanner buffer. All three fail quietly if they drift: an over-long line
+    /// is simply never read, a part that is not a whole number of base32 groups puts
+    /// every part after the first at the wrong offset, and one that is not a whole
+    /// number of words makes the device merge every part edge instead of writing it.
     #[test]
     fn the_default_part_is_the_largest_that_fits() {
         assert_eq!(
             DEFAULT_PART % 5,
             0,
             "base32 packs five bytes to eight chars"
         );
-        assert_eq!(fits(Encoding::Base32, SCANNER_BUFFER), DEFAULT_PART);
+        assert_eq!(DEFAULT_PART % 4, 0, "the device stages whole words");
         assert!(part_len(Encoding::Base32, DEFAULT_PART) <= SCANNER_BUFFER);
+        // The largest such size that fits: the next one up does not.
+        assert!(part_len(Encoding::Base32, DEFAULT_PART + 20) > SCANNER_BUFFER);
+        // And it is what base32 alone would allow, rounded down to a whole word.
+        assert_eq!(
+            fits(Encoding::Base32, SCANNER_BUFFER) / 20 * 20,
+            DEFAULT_PART
+        );
         // `Z` is base32 underneath, so the line arithmetic is the same either way.
         assert_eq!(
             part_len(Encoding::Zlib, DEFAULT_PART),