-
Notifications
You must be signed in to change notification settings - Fork 45
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
Signature alone is not enough to identify signed data #1238
Signature alone is not enough to identify signed data #1238
Conversation
but if we identify the signature and the signer, we can also identify the signed content.
Before this commit I saw a transaction failing with no messages.
624b2da
to
58beb0d
Compare
Codecov Report
@@ Coverage Diff @@
## master #1238 +/- ##
=======================================
Coverage 81.28% 81.28%
=======================================
Files 21 21
Lines 1453 1453
Branches 190 190
=======================================
Hits 1181 1181
Misses 231 231
Partials 41 41
Continue to review full report at Codecov.
|
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.
LGTM
These new fields were introduced in raiden-network/raiden-contracts#1238 to remove ambiguity. This is a part of raiden-network#254.
These new fields were introduced in raiden-network/raiden-contracts#1238 to remove ambiguity. This is a part of #254.
but if we identify the signature and the signer, we can also identify the signed content.
What this PR does
This PR adds
non_closing_address
andtoken_network_address
in RewardProofs.Why I'm making this PR
RewardProof contained a signature
non_closing_signature
and we relied on that to specify a call. However, when somebody changes the signed data, our signature verification algorithm just emits a different address. So a signature alone was not enough to specify a byte blob. We need the signer's address.Even when we can identify a byte blob, the same byte blob means different things on different
token_network_address
. Sotoken_network_address
is also added in the reward proof.What's tricky about this PR (if any)
We use a signature system to establish the integrity of (token_network_address, sig, address). Within
token_network_address
we use the signature system again onsig, address
. So the reasoning is a bit complicated.I expect the CI to fail now, but I'm seeking feedbacks now.
Any reviewer can check these:
make compile_contracts
and add the resultingraiden_contracts/data/contracts.json
in the PR.[skip ci]
in the commit message so Travis does not waste time.[skip ci]
and let Travis check the hash of the source.solidityFunction
_solidity_argument
solidity_variable
python_variable
PYTHON_CONSTANT
etherscan_verify.py
runs on the new contract.gas.json
.pytest.raises(TransactionFailed, match="error message"):
And before "merge" all checkboxes have to be checked. If you find redundant points, remove them.