Proper fix for optimized version of isBootstrapRedeemer function. See

#1937
IntersectMBO · Jan 25, 2022 · 9882747 · 9882747
1 parent cc843b3
commit 9882747
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 11 deletions.
diff --git a/eras/shelley/impl/src/Cardano/Ledger/Shelley/LedgerState.hs b/eras/shelley/impl/src/Cardano/Ledger/Shelley/LedgerState.hs
@@ -128,6 +128,7 @@ import Cardano.Ledger.Coin
     rationalToCoinViaFloor,
     toDeltaCoin,
   )
+import qualified Cardano.Ledger.CompactAddress as CA (isBootstrapRedeemer)
 import Cardano.Ledger.Compactible
 import Cardano.Ledger.Core (PParamsDelta)
 import qualified Cardano.Ledger.Core as Core
@@ -1666,8 +1667,15 @@ returnRedeemAddrsToReserves es = es {esAccountState = acnt', esLState = ls'}
     ls = esLState es
     us = _utxoState ls
     UTxO utxo = _utxo us
-    (redeemers, nonredeemers) =
-      SplitMap.partition (isBootstrapRedeemer . getTxOutAddr) utxo
+    -- TLDR we avoiding deserializing/serializing each address in the UTxO.
+    -- Here we can check very efficiently if an address is a Bootstrap redeemer,
+    -- by selecting an appropriate function depending on the current in-memory
+    -- representation of an address. If it is in compacted form we'll use the
+    -- specialized version that will only decompact an address if it is indeed a
+    -- Byron address. See #1937 for more info on the topic.
+    efficientPartition =
+      either isBootstrapRedeemer CA.isBootstrapRedeemer . getTxOutAddrEither
+    (redeemers, nonredeemers) = SplitMap.partition efficientPartition utxo
     acnt = esAccountState es
     utxoR = UTxO redeemers :: UTxO era
     acnt' =

diff --git a/eras/shelley/test-suite/src/Test/Cardano/Ledger/Shelley/Address/CompactAddr.hs b/eras/shelley/test-suite/src/Test/Cardano/Ledger/Shelley/Address/CompactAddr.hs
@@ -60,8 +60,7 @@ propDecompactShelleyLazyAddr = do
       keyHash1 = unsafeGetHash . CA.decompactAddr . mangle . CA.compactAddr $ addr
    in pure $ keyHash0 == keyHash1
 
--- | TODO This property test is failing to find a discrepancy that was found on mainnet.
-propIsBootstrapRedeemer :: Addr crypto -> Property
+propIsBootstrapRedeemer :: CC.Crypto crypto => Addr crypto -> Property
 propIsBootstrapRedeemer addr =
   Addr.isBootstrapRedeemer addr === CA.isBootstrapRedeemer (CA.compactAddr addr)
 

diff --git a/libs/cardano-ledger-core/src/Cardano/Ledger/CompactAddress.hs b/libs/cardano-ledger-core/src/Cardano/Ledger/CompactAddress.hs
@@ -32,6 +32,7 @@ import Cardano.Ledger.Address
     toWord7,
     word7sToWord64,
   )
+import qualified Cardano.Ledger.Address as Address (isBootstrapRedeemer)
 import Cardano.Ledger.BaseTypes (word8ToNetwork)
 import Cardano.Ledger.Credential
   ( Credential (KeyHashObj, ScriptHashObj),
@@ -229,12 +230,10 @@ getPayCred header = case testBit header payCredIsScript of
   True -> getScriptHash
   False -> getKeyHash
 
--- | WARNING: This optimized version of isBootstrapRedeemer does not agree
--- with the one in Cardano.Ledger.Address
-isBootstrapRedeemer :: CompactAddr crypto -> Bool
-isBootstrapRedeemer (UnsafeCompactAddr bytes) =
-  testBit header byron -- AddrBootstrap
-    && addrType == 2 -- ATRedeem
+-- | Optimized version of `Address.isBootstrapRedeemer`, where we avoid
+-- decompacting of post Byron addresses.
+isBootstrapRedeemer :: forall crypto. CC.Crypto crypto => CompactAddr crypto -> Bool
+isBootstrapRedeemer cAddr@(UnsafeCompactAddr bytes) =
+  testBit header byron && Address.isBootstrapRedeemer (decompactAddr cAddr :: Addr crypto)
   where
-    addrType = SBS.index bytes (SBS.length bytes - 6)
     header = SBS.index bytes 0