Fully internalise balancing, fix tests #48
Conversation
| Balance.balanceTxStep minUtxo fees utxoIndex ownPkh tx | ||
|
|
||
| txInputs <$> prebalancedTx @?= Right (Set.fromList [txIn1, txIn2, txIn3]) | ||
| txInputs <$> balancedTx @?= Right (Set.fromList [txIn1, txIn2]) |
There was a problem hiding this comment.
Note this input was removed, unsure why it was ever there?
The expected output of this tx is 1.6ADA + 0.5ADA fee, inputs 1 and 2 are 1.1ADA and 1.0ADA, both summing to 2.1ADA, so only 2 inputs should be needed to cover it.
There was a problem hiding this comment.
I think this is the edge case we talked about, when even though the inputs and outputs are balanced, the cli adds a change utxo which must be at least the minUtxo (around 1 ada). This is now outdated.
I'm wondering though what happens if we change the amout being sent to 1.5, so the whole amount is 2ADA, with a change of 0.1, which then must be rounded up.
| , | ||
| ( 6 | ||
| , -- Steps 4, 5 and 6 are near repeats of 1, 2 and 3, to ensure min utxo values are met | ||
|
|
There was a problem hiding this comment.
Fourmolu decided this space should be here, not sure why
| commandEqual :: Text -> Text -> Bool | ||
| commandEqual "" "" = True | ||
| commandEqual "" _ = False | ||
| commandEqual _ "" = False | ||
| commandEqual expected actual = maybe False (on commandEqual dropToSpace postExp) mPostAct | ||
| where | ||
| (preExp, postExp) = Text.breakOn "?" expected | ||
| mPostAct = Text.stripPrefix preExp actual | ||
| dropToSpace = Text.dropWhile (/= ' ') |
There was a problem hiding this comment.
This could likely be done better, didn't want to bring all of regex in though, any ideas?
There was a problem hiding this comment.
I can't come up with anything better right how either.
However, I think we could have the trimming of newlines on line 875 merged into this function
There was a problem hiding this comment.
Oh yeah, good point
szg251
left a comment
szg251
left a comment
There was a problem hiding this comment.
Nice to see that actually the code got simpler
| Balance.balanceTxStep minUtxo fees utxoIndex ownPkh tx | ||
|
|
||
| txInputs <$> prebalancedTx @?= Right (Set.fromList [txIn1, txIn2, txIn3]) | ||
| txInputs <$> balancedTx @?= Right (Set.fromList [txIn1, txIn2]) |
There was a problem hiding this comment.
I think this is the edge case we talked about, when even though the inputs and outputs are balanced, the cli adds a change utxo which must be at least the minUtxo (around 1 ada). This is now outdated.
I'm wondering though what happens if we change the amout being sent to 1.5, so the whole amount is 2ADA, with a change of 0.1, which then must be rounded up.
| commandEqual :: Text -> Text -> Bool | ||
| commandEqual "" "" = True | ||
| commandEqual "" _ = False | ||
| commandEqual _ "" = False | ||
| commandEqual expected actual = maybe False (on commandEqual dropToSpace postExp) mPostAct | ||
| where | ||
| (preExp, postExp) = Text.breakOn "?" expected | ||
| mPostAct = Text.stripPrefix preExp actual | ||
| dropToSpace = Text.dropWhile (/= ' ') |
There was a problem hiding this comment.
I can't come up with anything better right how either.
However, I think we could have the trimming of newlines on line 875 merged into this function
Improve tests to catch above
|
Following a real world test, I've updated the tests to now scale fee as inputs/outputs update. This exposed a fault with the ada balancing, I've fixed this with some unfortunately not super beautiful code. |
|
balancing is hard... |
szg251
left a comment
szg251
left a comment
There was a problem hiding this comment.
TBH, I'm happy with this as it is, as refactoring balancing might not worth the trouble for now.
|
Tested with the transfer example - all good. |
This interalises ada balancing, no longer relying on
cardano-cli transaction build, and always building raw.This means the Tx returned from
submitTxorbalanceTxis identical to the one submitted to the chain.It also tweaks the balancing loop a bit to be more correct.
As such,
PreBalancehas been renamed toBalanceIn order to handle the now changing txid of transaction as they are balanced, the tests were changed to now support a "?" in assertCommandHistory. Anywhere where a "?" is used in an expected command is seen as identical to up to the next space in the actual command.
See it as a regex
^[^ ]+(?= ).e.g.
As an actual command would compare equal to: