From 43231f8da11548cf2e6a548efd29aaf0810fdd4c Mon Sep 17 00:00:00 2001 From: Moremi Vannak Date: Tue, 25 May 2021 20:43:09 +0700 Subject: [PATCH 1/2] [#257] Add empty transactions test Problem: Both `treasuryDAO` and `registryDAO` have proposals that can support xtz transfers. Both implement `handle_transfer` for `Xtz_transfer_type` in a way that can fail: empty transactions (of `unit` parameter an `0` amount) will be rejected by the network. Solution: Add an emptry transactions test to prove this. --- haskell/test/Test/Ligo/RegistryDAO.hs | 21 +++++++++++ haskell/test/Test/Ligo/TreasuryDAO.hs | 50 +++++++++++++++++++++++---- src/registryDAO.mligo | 2 +- src/treasuryDAO.mligo | 2 +- 4 files changed, 66 insertions(+), 9 deletions(-) diff --git a/haskell/test/Test/Ligo/RegistryDAO.hs b/haskell/test/Test/Ligo/RegistryDAO.hs index 862db5d0..baae6392 100644 --- a/haskell/test/Test/Ligo/RegistryDAO.hs +++ b/haskell/test/Test/Ligo/RegistryDAO.hs @@ -146,6 +146,27 @@ test_RegistryDAO = baseDao (Call @"Propose") (ProposeParams proposalSize proposalMeta) & expectFailProposalCheck baseDao + , nettestScenarioCaps "proposal_check: fail when xtz transfer contains 0 mutez" $ + withOriginated 2 + (\(admin: wallet1:_) -> + initialStorageWithExplictRegistryDAOConfig admin [wallet1] + & setExtra @Natural [mt|min_xtz_amount|] 0 + ) $ + \(_:wallet1:_) _ baseDao _ -> do + let proposalMeta = lPackValueRaw @RegistryDaoProposalMetadata $ + Transfer_proposal $ + TransferProposal 1 [ xtzTransferType 0 wallet1 ] [] + let proposalSize = metadataSize proposalMeta + withSender wallet1 $ + call baseDao (Call @"Freeze") (#amount .! proposalSize) + + -- Advance one voting period to a proposing stage. + advanceTime (sec $ 11) + + withSender wallet1 $ call baseDao (Call @"Propose") + (ProposeParams proposalSize proposalMeta) + & expectFailProposalCheck baseDao + , nettestScenarioCaps "checks it fails if required tokens are not frozen" $ withOriginated 2 (\(admin: wallet1:_) -> initialStorageWithExplictRegistryDAOConfig admin [wallet1]) $ diff --git a/haskell/test/Test/Ligo/TreasuryDAO.hs b/haskell/test/Test/Ligo/TreasuryDAO.hs index a24dfdfe..c02db581 100644 --- a/haskell/test/Test/Ligo/TreasuryDAO.hs +++ b/haskell/test/Test/Ligo/TreasuryDAO.hs @@ -50,8 +50,13 @@ test_TreasuryDAO = testGroup "TreasuryDAO Tests" \_emulated -> flushTokenTransfer , nettestScenarioOnEmulator "can flush a Xtz transfer proposal" $ \_emulated -> flushXtzTransfer + ] + + , testGroup "proposal_check:" + [ nettestScenarioOnEmulator "fail when xtz transfer contains 0 mutez" $ + \_emulated -> proposalCheckFailZeroMutez - -- -- TODO #260: check all fail scenario of proposal_check + -- TODO #260: check all fail scenario of proposal_check ] ] @@ -62,7 +67,7 @@ metadataSize md = fromIntegral $ BS.length md validProposal :: HasCallStack => NettestScenario m validProposal = uncapsNettest $ withFrozenCallStack do DaoOriginateData{..} <- - originateTreasuryDaoWithBalance $ \owner1_ owner2_ -> + originateTreasuryDaoWithBalance id $ \owner1_ owner2_ -> [ ((owner1_, frozenTokenId), 200) , ((owner2_, frozenTokenId), 200) ] @@ -93,7 +98,7 @@ validProposal = uncapsNettest $ withFrozenCallStack do flushTokenTransfer :: HasCallStack => NettestScenario m flushTokenTransfer = uncapsNettest $ withFrozenCallStack $ do DaoOriginateData{..} <- - originateTreasuryDaoWithBalance $ \_ owner2_ -> + originateTreasuryDaoWithBalance id $ \_ owner2_ -> [ ((owner2_, frozenTokenId), 100) ] @@ -140,7 +145,7 @@ flushTokenTransfer = uncapsNettest $ withFrozenCallStack $ do flushXtzTransfer :: HasCallStack => NettestScenario m flushXtzTransfer = uncapsNettest $ withFrozenCallStack $ do DaoOriginateData{..} <- - originateTreasuryDaoWithBalance $ \_ _ -> + originateTreasuryDaoWithBalance id $ \_ _ -> [] sendXtz (toAddress dodDao) (unsafeBuildEpName "callCustom") ([mt|receive_xtz|], lPackValueRaw ()) @@ -190,7 +195,36 @@ flushXtzTransfer = uncapsNettest $ withFrozenCallStack $ do advanceTime (sec 10) withSender dodAdmin $ call dodDao (Call @"Flush") 100 --- -- TODO: check xtz balance + -- TODO: check xtz balance + + +proposalCheckFailZeroMutez :: HasCallStack => NettestScenario m +proposalCheckFailZeroMutez = uncapsNettest $ withFrozenCallStack do + DaoOriginateData{..} <- + originateTreasuryDaoWithBalance + (\store -> setExtra @Natural [mt|min_xtz_amount|] 0 store) $ + \owner1_ owner2_ -> + [ ((owner1_, frozenTokenId), 200) + , ((owner2_, frozenTokenId), 200) + ] + let + proposalMeta = lPackValueRaw @TreasuryDaoProposalMetadata $ + TransferProposal + { tpAgoraPostId = 1 + , tpTransfers = [ xtzTransferType 0 dodOwner2 ] + } + proposalSize = metadataSize proposalMeta + + -- Freeze in voting stage. + withSender dodOwner1 $ + call dodDao (Call @"Freeze") (#amount .! proposalSize) + + -- Advance one voting period to a proposing stage. + advanceTime (sec 10) + + withSender dodOwner1 $ + call dodDao (Call @"Propose") (ProposeParams proposalSize proposalMeta) + & expectCustomErrorNoArg #fAIL_PROPOSAL_CHECK dodDao -------------------------------------------------------------------------- -- Helper @@ -218,8 +252,9 @@ tokenTransferType contractAddr fromAddr toAddr = Token_transfer_type TokenTransf originateTreasuryDaoWithBalance :: forall caps base m. (MonadNettest caps base m) - => (Address -> Address -> [(LedgerKey, LedgerValue)]) -> OriginateFn m -originateTreasuryDaoWithBalance bal = + => (FullStorage -> FullStorage) + -> (Address -> Address -> [(LedgerKey, LedgerValue)]) -> OriginateFn m +originateTreasuryDaoWithBalance modifyStorageFn bal = let fs = fromVal ($(fetchValue @FullStorage "haskell/test/treasuryDAO_storage.tz" "TREASURY_STORAGE_PATH")) FullStorage'{..} = fs & setExtra @Natural [mt|frozen_scale_value|] 1 @@ -229,6 +264,7 @@ originateTreasuryDaoWithBalance bal = & setExtra @Natural [mt|max_proposal_size|] 1000 & setExtra @Natural [mt|min_xtz_amount|] 2 & setExtra @Natural [mt|max_xtz_amount|] 5 + & modifyStorageFn in originateLigoDaoWithBalance (sExtra fsStorage) (fsConfig diff --git a/src/registryDAO.mligo b/src/registryDAO.mligo index 799f127e..242ce258 100644 --- a/src/registryDAO.mligo +++ b/src/registryDAO.mligo @@ -72,7 +72,7 @@ let registry_DAO_proposal_check (params, extras : propose_params * contract_extr let is_all_transfers_valid (is_valid, transfer_type: bool * transfer_type) = match transfer_type with | Token_transfer_type _tt -> is_valid - | Xtz_transfer_type xt -> is_valid && min_xtz_amount <= xt.amount && xt.amount <= max_xtz_amount + | Xtz_transfer_type xt -> min_xtz_amount <= xt.amount && xt.amount <= max_xtz_amount in List.fold is_all_transfers_valid tp.transfers has_correct_token_lock | Update_receivers_proposal _urp -> has_correct_token_lock diff --git a/src/treasuryDAO.mligo b/src/treasuryDAO.mligo index c6446b95..7bafde25 100644 --- a/src/treasuryDAO.mligo +++ b/src/treasuryDAO.mligo @@ -32,7 +32,7 @@ let treasury_DAO_proposal_check (params, extras : propose_params * contract_extr let is_all_transfers_valid (is_valid, transfer_type: bool * transfer_type) = match transfer_type with | Token_transfer_type _tt -> is_valid - | Xtz_transfer_type xt -> is_valid && min_xtz_amount <= xt.amount && xt.amount <= max_xtz_amount + | Xtz_transfer_type xt -> min_xtz_amount <= xt.amount && xt.amount <= max_xtz_amount in List.fold is_all_transfers_valid pm.transfers true else From 77a0f135fa84ec3db422fe084094c75728beebb8 Mon Sep 17 00:00:00 2001 From: Moremi Vannak Date: Wed, 26 May 2021 17:51:03 +0700 Subject: [PATCH 2/2] [#257] Prevent empty transactions in transfer proposals Problem: Since empty transactions does now throw error as expect, we need to update baseDAO to throw this error. Solution: Update registry and treasury dao to fail proposal check when it contains empty transactions (0 mutez xtz transfer). --- src/registryDAO.mligo | 4 +++- src/treasuryDAO.mligo | 4 +++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/registryDAO.mligo b/src/registryDAO.mligo index 242ce258..8c8d1e0f 100644 --- a/src/registryDAO.mligo +++ b/src/registryDAO.mligo @@ -72,7 +72,9 @@ let registry_DAO_proposal_check (params, extras : propose_params * contract_extr let is_all_transfers_valid (is_valid, transfer_type: bool * transfer_type) = match transfer_type with | Token_transfer_type _tt -> is_valid - | Xtz_transfer_type xt -> min_xtz_amount <= xt.amount && xt.amount <= max_xtz_amount + | Xtz_transfer_type xt -> + is_valid && xt.amount <> 0mutez + && min_xtz_amount <= xt.amount && xt.amount <= max_xtz_amount in List.fold is_all_transfers_valid tp.transfers has_correct_token_lock | Update_receivers_proposal _urp -> has_correct_token_lock diff --git a/src/treasuryDAO.mligo b/src/treasuryDAO.mligo index 7bafde25..be457334 100644 --- a/src/treasuryDAO.mligo +++ b/src/treasuryDAO.mligo @@ -32,7 +32,9 @@ let treasury_DAO_proposal_check (params, extras : propose_params * contract_extr let is_all_transfers_valid (is_valid, transfer_type: bool * transfer_type) = match transfer_type with | Token_transfer_type _tt -> is_valid - | Xtz_transfer_type xt -> min_xtz_amount <= xt.amount && xt.amount <= max_xtz_amount + | Xtz_transfer_type xt -> + is_valid && xt.amount <> 0mutez + && min_xtz_amount <= xt.amount && xt.amount <= max_xtz_amount in List.fold is_all_transfers_valid pm.transfers true else