-
Notifications
You must be signed in to change notification settings - Fork 631
add extra guard to fail if trying to submit a too big transaction #4131
Conversation
This can happen because we only _estimate_ the maximum number of inputs allowed for the coin selection. As a result, the transaction generated as a result of the coin selection may be slightly bigger than the maximum sized allowed by the network (currently 8kb). So, having an extra sanity checks prevent wrong transactions to be submitted (and added to our pendin set)
Right meta -> return $ Right (taTx $ txAux, meta) | ||
let sz = fromIntegral $ BL.length $ serialize txAux | ||
maxSz <- Node.getMaxTxSize (walletPassive activeWallet ^. walletNode) | ||
if sz > maxSz then |
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.
@KtorZ, just to make sure, we don't need sz >= maxSz
here (most probably not, but asking just to be safe :) )
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.
I am not sure how cardano-sl approaches maximum actually and whether it treats it as an inclusive or exclusive boundary 🤔 ...
@@ -177,7 +177,7 @@ pay activeWallet spendingPassword opts accountId payees = do | |||
Right (txAux, partialMeta, _utxo) -> do | |||
let sz = fromIntegral $ BL.length $ serialize txAux | |||
maxSz <- Node.getMaxTxSize (walletPassive activeWallet ^. walletNode) | |||
if sz > maxSz then | |||
if sz >= maxSz then |
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.
👌
@piotr-iohk can you approve the PR so I can merge it? |
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.
👍
bors r+ |
4131: add extra guard to fail if trying to submit a too big transaction r=disassembler a=KtorZ ## Description <!--- A brief description of this PR and the problem is trying to solve --> This can happen because we only _estimate_ the maximum number of inputs allowed for the coin selection. As a result, the transaction generated as a result of the coin selection may be slightly bigger than the maximum sized allowed by the network (currently 8kb). So, having an extra sanity checks prevent wrong transactions to be submitted (and added to our pendin set) ## Linked issue <!--- Put here the relevant issue from YouTrack --> ? Co-authored-by: KtorZ <matthias.benkort@gmail.com> Co-authored-by: Samuel Leathers <samuel.leathers@iohk.io>
bors r+ |
4131: add extra guard to fail if trying to submit a too big transaction r=disassembler a=KtorZ ## Description <!--- A brief description of this PR and the problem is trying to solve --> This can happen because we only _estimate_ the maximum number of inputs allowed for the coin selection. As a result, the transaction generated as a result of the coin selection may be slightly bigger than the maximum sized allowed by the network (currently 8kb). So, having an extra sanity checks prevent wrong transactions to be submitted (and added to our pendin set) ## Linked issue <!--- Put here the relevant issue from YouTrack --> ? Co-authored-by: KtorZ <matthias.benkort@gmail.com> Co-authored-by: Samuel Leathers <samuel.leathers@iohk.io>
Build succeeded |
Description
This can happen because we only estimate the maximum number of inputs allowed for the coin selection.
As a result, the transaction generated as a result of the coin selection may be slightly bigger than
the maximum sized allowed by the network (currently 8kb). So, having an extra sanity checks prevent
wrong transactions to be submitted (and added to our pendin set)
Linked issue
?
Type of change
Developer checklist
Testing checklist
QA Steps
Screenshots (if available)
How to merge
Send the message
bors r+
to merge this PR. For more information, seedocs/how-to/bors.md
.