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.

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

add ErrInvalidTx to capture Quantity=0 situation in Byron (http-bridge) #445

Merged
merged 6 commits into from Jun 21, 2019

Conversation

paweljakubas
Copy link
Contributor

@paweljakubas paweljakubas commented Jun 20, 2019

Issue Number

#364

Overview

  • I have added the test illustrating the case
  • I have added ErrInvalidTx to transaction layer and expose it with http-bridge case only

Comments

@paweljakubas paweljakubas self-assigned this Jun 20, 2019
@paweljakubas paweljakubas force-pushed the paweljakubas/364/transaction-with-0-errors branch 2 times, most recently from d797869 to 284e8b2 Compare June 20, 2019 14:11
@paweljakubas paweljakubas changed the title add Quantity=0 test illustrating expected 403 error add ErrInvalidTx to capture Quantity=0 situation in Byron (http-bridge) Jun 20, 2019
= ErrKeyNotFoundForAddress Address
-- ^ We tried to sign a transaction with inputs that are unknown to us?
| ErrInvalidTx
-- ^ when transaction with 0 amount is tried (not valid in Shelly)
Copy link
Member

Choose a reason for hiding this comment

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

Comment is wrong: Shelly --> Byron

Copy link
Contributor Author

Choose a reason for hiding this comment

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

indeed, corrected

@@ -52,10 +54,12 @@ newTransactionLayer = TransactionLayer
{ mkStdTx = \keyFrom inps outs -> do
let ins = (fmap fst inps)
let tx = Tx ins outs
unless (not (any (\ (TxOut _ c) -> c == Coin 0) outs))
Copy link
Member

Choose a reason for hiding this comment

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

when (any ...) is a bit less-confusing than the double-negation "unless (not (any" I think 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, I had when (not. null ... which is more natural to me, but hlint suggested current version. I used , when (any ..) in the end

-- Not working, maybe we need to make TransactionLayer polymorphic
let txSigData = txId @(HttpBridge n) tx
txWitnesses <- forM inps $ \(_in, TxOut addr _c) -> mkWitness txSigData
<$> withEither (ErrKeyNotFoundForAddress addr) (keyFrom addr)
<$> maybeToRight (ErrKeyNotFoundForAddress addr) (keyFrom addr)
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -220,6 +221,10 @@ errMsg403NotEnoughMoney has needs = "I can't process this payment because there'
\ " ++ show has ++ " Lovelace, but I need " ++ show needs ++ " Lovelace\
\ (excluding fee amount) in order to proceed with the payment."

errMsg403InvalidTransaction :: String
errMsg403InvalidTransaction = "I can't process this payment because transactions\
\ with 0 amount are not supported in Byron."
Copy link
Member

Choose a reason for hiding this comment

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

Wording suggestion:

I can't process this payment because it contains at least one payment output of value 0. This isn't supported by the current core nodes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@paweljakubas paweljakubas force-pushed the paweljakubas/364/transaction-with-0-errors branch from f31da85 to a1281c3 Compare June 20, 2019 14:24
@KtorZ KtorZ force-pushed the paweljakubas/364/transaction-with-0-errors branch from d96cab2 to d61a812 Compare June 21, 2019 16:20
@KtorZ KtorZ merged commit d18adfb into master Jun 21, 2019
@KtorZ KtorZ deleted the paweljakubas/364/transaction-with-0-errors branch June 21, 2019 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants