From fe1676bc3a67b52cacfc8bbc25cd0197cb4c408c Mon Sep 17 00:00:00 2001
From: Tyera Eulberg <tyera@solana.com>
Date: Mon, 11 Mar 2019 16:35:25 -0600
Subject: [PATCH] Review comments

---
 programs/budget/src/lib.rs |  5 +++--
 programs/system/src/lib.rs |  4 ++--
 programs/token/src/lib.rs  |  9 +++++----
 runtime/src/runtime.rs     | 32 +++++++++++++++++++++++++-------
 sdk/src/native_program.rs  | 14 --------------
 5 files changed, 35 insertions(+), 29 deletions(-)

diff --git a/programs/budget/src/lib.rs b/programs/budget/src/lib.rs
index a5c4759d2d..81c824c3ed 100644
--- a/programs/budget/src/lib.rs
+++ b/programs/budget/src/lib.rs
@@ -1,11 +1,12 @@
 mod budget_program;
 
 use crate::budget_program::process_instruction;
+use bincode::serialize;
 use log::*;
 use solana_sdk::account::KeyedAccount;
 use solana_sdk::native_program::ProgramError;
 use solana_sdk::pubkey::Pubkey;
-use solana_sdk::{custom_error, solana_entrypoint};
+use solana_sdk::solana_entrypoint;
 
 solana_entrypoint!(entrypoint);
 fn entrypoint(
@@ -19,5 +20,5 @@ fn entrypoint(
     trace!("process_instruction: {:?}", data);
     trace!("keyed_accounts: {:?}", keyed_accounts);
     process_instruction(program_id, keyed_accounts, data)
-        .map_err(|e| ProgramError::CustomError(custom_error!(e)))
+        .map_err(|e| ProgramError::CustomError(serialize(&e).unwrap()))
 }
diff --git a/programs/system/src/lib.rs b/programs/system/src/lib.rs
index 163777d018..eced0db5bb 100644
--- a/programs/system/src/lib.rs
+++ b/programs/system/src/lib.rs
@@ -1,7 +1,7 @@
+use bincode::serialize;
 use log::*;
 use serde_derive::Serialize;
 use solana_sdk::account::KeyedAccount;
-use solana_sdk::custom_error;
 use solana_sdk::native_program::ProgramError;
 use solana_sdk::pubkey::Pubkey;
 use solana_sdk::system_instruction::SystemInstruction;
@@ -101,7 +101,7 @@ pub fn entrypoint(
                     SystemError::ResultWithNegativeLamports => {
                         ProgramError::ResultWithNegativeLamports
                     }
-                    e => ProgramError::CustomError(custom_error!(e)),
+                    e => ProgramError::CustomError(serialize(&e).unwrap()),
                 }
             }),
             SystemInstruction::Assign { program_id } => {
diff --git a/programs/token/src/lib.rs b/programs/token/src/lib.rs
index 97ea8fb766..7ec8140b6c 100644
--- a/programs/token/src/lib.rs
+++ b/programs/token/src/lib.rs
@@ -1,8 +1,9 @@
+use bincode::serialize;
 use log::*;
 use solana_sdk::account::KeyedAccount;
 use solana_sdk::native_program::ProgramError;
 use solana_sdk::pubkey::Pubkey;
-use solana_sdk::{custom_error, solana_entrypoint};
+use solana_sdk::solana_entrypoint;
 
 mod token_program;
 
@@ -15,8 +16,8 @@ fn entrypoint(
 ) -> Result<(), ProgramError> {
     solana_logger::setup();
 
-    token_program::TokenProgram::process(program_id, info, input).map_err(|err| {
-        error!("error: {:?}", err);
-        ProgramError::CustomError(custom_error!(err))
+    token_program::TokenProgram::process(program_id, info, input).map_err(|e| {
+        error!("error: {:?}", e);
+        ProgramError::CustomError(serialize(&e).unwrap())
     })
 }
diff --git a/runtime/src/runtime.rs b/runtime/src/runtime.rs
index 3bf2254c37..26c0685286 100644
--- a/runtime/src/runtime.rs
+++ b/runtime/src/runtime.rs
@@ -108,13 +108,7 @@ fn execute_instruction(
         program_accounts,
         tick_height,
     )
-    .map_err(|err| match err {
-        ProgramError::CustomError(mut error) => {
-            error.truncate(32);
-            ProgramError::CustomError(error)
-        }
-        e => e,
-    })?;
+    .map_err(verify_error)?;
 
     // Verify the instruction
     for ((pre_program_id, pre_lamports, pre_userdata), post_account) in
@@ -233,6 +227,16 @@ where
     Ok(())
 }
 
+fn verify_error(err: ProgramError) -> ProgramError {
+    match err {
+        ProgramError::CustomError(mut error) => {
+            error.truncate(32);
+            ProgramError::CustomError(error)
+        }
+        e => e,
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
@@ -312,4 +316,18 @@ mod tests {
         );
     }
 
+    #[test]
+    fn test_verify_error() {
+        let short_error = ProgramError::CustomError(vec![1, 2, 3]);
+        let expected_short_error = short_error.clone(); // short CustomError errors should be untouched
+        assert_eq!(verify_error(short_error), expected_short_error);
+
+        let long_error = ProgramError::CustomError(vec![8; 40]);
+        let expected_long_error = ProgramError::CustomError(vec![8; 32]); // long CustomError errors should be truncated
+        assert_eq!(verify_error(long_error), expected_long_error);
+
+        let other_error = ProgramError::GenericError;
+        let expected_other_error = other_error.clone(); // non-CustomError errors should be untouched
+        assert_eq!(verify_error(other_error), expected_other_error);
+    }
 }
diff --git a/sdk/src/native_program.rs b/sdk/src/native_program.rs
index ebebc23a51..6c9e8e548d 100644
--- a/sdk/src/native_program.rs
+++ b/sdk/src/native_program.rs
@@ -51,20 +51,6 @@ impl std::fmt::Display for ProgramError {
 }
 impl std::error::Error for ProgramError {}
 
-// Convenience macro to serialize (and potentially truncate) a program-specific error to pass in
-// ProgramError::CustomError
-#[macro_export]
-macro_rules! custom_error(
-    ($program_error:expr) => ({
-        use bincode::serialize;
-        let mut error = serialize(&$program_error).expect("failed to serialize program error");
-        if error.len() > 32 {
-            error.truncate(32);
-        }
-        error
-    });
-);
-
 // All native programs export a symbol named process()
 pub const ENTRYPOINT: &str = "process";