Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Move estimateTxSize etc to separate module #4036

Merged
merged 3 commits into from
Jul 14, 2023
Merged

Move estimateTxSize etc to separate module #4036

merged 3 commits into from
Jul 14, 2023

Conversation

Anviking
Copy link
Member

@Anviking Anviking commented Jul 11, 2023 •
edited
Loading

Move to the following module:

module Cardano.Wallet.Write.Tx.Balance.CoinSelection
    ( -- * Coin-selection for use with balanceTx
      -- TODO: Move the actual coin-selection function to here, and don't expose
      -- this group of functions / types.
      estimateTxSize
    , estimateTxCost
    , TxSkeleton (..)

     -- * Additional exposed functionality
     -- ** For migration
    , txConstraints

     -- ** For estimateSignedTxSize
    , sizeOf_BootstrapWitnesses

      -- ** For the wallet
    , _txRewardWithdrawalCost
    , getFeePerByteFromWalletPParams
    )

Comments

Depends on #4030

Issue Number

ADP-3081

@Anviking Anviking self-assigned this Jul 11, 2023
@Anviking Anviking changed the base branch from master to anviking/ADP-3053/move-assignScriptRedeemers July 11, 2023 15:36
@Anviking Anviking force-pushed the anviking/ADP-3081/move-estimateTxSize branch from b38e5e5 to 9408e10 Compare July 11, 2023 15:48
@Anviking Anviking force-pushed the anviking/ADP-3053/move-assignScriptRedeemers branch from 895fec3 to e032182 Compare July 11, 2023 16:08
@Anviking Anviking force-pushed the anviking/ADP-3081/move-estimateTxSize branch from 9408e10 to 04640b0 Compare July 11, 2023 16:16
Base automatically changed from anviking/ADP-3053/move-assignScriptRedeemers to master July 12, 2023 08:57
@Anviking Anviking force-pushed the anviking/ADP-3081/move-estimateTxSize branch 2 times, most recently from 0be0020 to 04fe1b8 Compare July 12, 2023 11:41
@Anviking Anviking force-pushed the anviking/ADP-3081/move-estimateTxSize branch from 04fe1b8 to 5b925cf Compare July 12, 2023 11:42
@Anviking Anviking marked this pull request as ready for review July 12, 2023 11:44
@Anviking Anviking changed the title Move estimateTxSize etc to separate module Move estimateTxSize etc to separate module Jul 12, 2023
@Anviking Anviking mentioned this pull request Jul 12, 2023
Merged
@Anviking Anviking force-pushed the anviking/ADP-3081/move-estimateTxSize branch from 7b24f1f to 16b3ce1 Compare July 13, 2023 11:53
jonathanknowles
jonathanknowles approved these changes Jul 14, 2023
View reviewed changes
Copy link
Member

@jonathanknowles jonathanknowles left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Anviking

This looks good! Though I do have some suggestions and comments.

lib/wallet/src/Cardano/Wallet/Shelley/Transaction.hs Outdated Show resolved Hide resolved
lib/wallet/src/Cardano/Wallet/Write/Tx/SizeEstimation.hs Outdated Show resolved Hide resolved
lib/wallet/src/Cardano/Wallet/Write/Tx/SizeEstimation.hs Outdated Show resolved Hide resolved
lib/wallet/src/Cardano/Wallet/Write/Tx/SizeEstimation.hs Outdated Show resolved Hide resolved
lib/wallet/src/Cardano/Wallet/Write/Tx/SizeEstimation.hs Outdated Show resolved Hide resolved
lib/wallet/src/Cardano/Wallet/Write/Tx/SizeEstimation.hs Outdated Show resolved Hide resolved
lib/wallet/src/Cardano/Wallet/Write/Tx/SizeEstimation.hs Outdated Show resolved Hide resolved
lib/wallet/src/Cardano/Wallet/Write/Tx/SizeEstimation.hs Outdated Show resolved Hide resolved
lib/wallet/src/Cardano/Wallet/Write/Tx/SizeEstimation.hs Outdated Show resolved Hide resolved
lib/wallet/src/Cardano/Wallet/Write/Tx/SizeEstimation.hs Outdated Show resolved Hide resolved
@Anviking @jonathanknowles
Apply suggestions from code review
af58245 
Co-authored-by: Jonathan Knowles <mail@jonathanknowles.net>
@Anviking Anviking force-pushed the anviking/ADP-3081/move-estimateTxSize branch from 5e90fed to af58245 Compare July 14, 2023 09:49
@Anviking Anviking enabled auto-merge July 14, 2023 10:22
@Anviking
Copy link
Member Author

Thanks @jonathanknowles!

@Anviking Anviking added this pull request to the merge queue Jul 14, 2023
Merged via the queue into master with commit 015e6be Jul 14, 2023
2 checks passed
@Anviking Anviking deleted the anviking/ADP-3081/move-estimateTxSize branch July 14, 2023 11:04
github-merge-queue bot pushed a commit that referenced this pull request Jul 20, 2023
@Anviking
Move estimateSignedTxSize to separate module (#4037)
40fb32f 
```haskell
-- |
-- Copyright: © 2023 IOHK, 2023 Cardano Foundation
-- License: Apache-2.0
--
-- Module for 'signTx' and signing-related utilities for balancing.
module Cardano.Wallet.Write.Tx.Sign
    (
    -- * Signing transactions
    -- TODO: Move signTx function here

    -- * Signing-related utilities required for balancing
      estimateSignedTxSize

    , KeyWitnessCount (..)
    , estimateKeyWitnessCount
    )
    where
```

### Comments

- Depends on #4036

### Issue Number

ADP-3081
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonathanknowles jonathanknowles jonathanknowles approved these changes

Assignees

@Anviking Anviking

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Anviking @jonathanknowles