This repository was archived by the owner on Aug 18, 2020. It is now read-only.
[RCD-40] Enforce address discrimination check when sending transactions #3823
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
KtorZ
changed the title
KtorZ
changed the title
KtorZ
commented
Nov 5, 2018
intricate
force-pushed
the
KtorZ/RCD-40/enforce-address-discrimination-when-sending-tx
branch
2 times, most recently
from
a502bb8 to
d311d12
Compare
KtorZ
commented
Nov 5, 2018
| genPayeeWithNM :: NetworkMagic -> Core.Utxo -> Pay -> Gen (NonEmpty Core.TxOut) | ||
| genPayeeWithNM nm _utxo payment = do | ||
| let balance = toLovelaces payment | ||
| genTxOutWithNM nm StakeGenOptions { |
There was a problem hiding this comment.
Genuine question: I thought stake distribution wasn't used in cardano addresses?
There was a problem hiding this comment.
Is it important to get an answer to this?
|
LGTM. I can't approve on github because I created the PR 🙃 |
intricate
requested review from
erikd,
mhuesch,
intricate and
alan-mcnicholas
and removed request for
intricate
intricate
force-pushed
the
KtorZ/RCD-40/enforce-address-discrimination-when-sending-tx
branch
2 times, most recently
from
6bbe086 to
b51501a
Compare
kderme
reviewed
Nov 6, 2018
KtorZ
force-pushed
the
KtorZ/RCD-40/enforce-address-discrimination-when-sending-tx
branch
from
f06a813 to
1ce9786
Compare
…rkMagic` in `wallet-new` API V1 (cherry picked from commit a6373f6)
Both changes (new tests for fees sanity check and network magic validation) were introduced in parallel and merged concomitanttly. As a result, CI accepted both changes whereas the newly introduced tests should also be fixed.
KtorZ
force-pushed
the
KtorZ/RCD-40/enforce-address-discrimination-when-sending-tx
branch
from
1ce9786 to
c91aafa
Compare
|
*Fixed invalid tests newly introduced by CSL-2526 which also need to account for NetworkMagic when generating new payee. |
intricate
approved these changes
Nov 6, 2018
mhuesch
approved these changes
Nov 6, 2018
KtorZ
deleted the
KtorZ/RCD-40/enforce-address-discrimination-when-sending-tx
branch
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
On release/2.0.0, one can attempt to construct and send transactions to addresses which are of a different network via the wallet-new V1 API. Even though the nodes reject this transaction since it's invalid (as a result of CO-410), an error should be returned by the API in order to prevent users from even attempting this.
Linked issue
[RCD-40]
Type of change
Developer checklist
Testing checklist
QA Steps
Screenshots (if available)