-
Notifications
You must be signed in to change notification settings - Fork 213
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
Conversation
d797869
to
284e8b2
Compare
= 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) |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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)) |
There was a problem hiding this comment.
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 😄
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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." |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
f31da85
to
a1281c3
Compare
nix-tools patch add test illustrating the case
correct comment
d96cab2
to
d61a812
Compare
Issue Number
#364
Overview
Comments