Reduce max length of signature in vote message

[djrtwo]

Issue

Vote messages must be less that or equal 1024 bytes, defined by the type in the vote method signature. When parsing the signature of the vote from the vote message, we currently only enforce that the signature too is less than or equal to 1024 bytes.

casper/casper/contracts/simple_casper.v.py

Line 402 in d5aed93

sig: bytes <= 1024 = values[4]

Due to the variable amount of bytes required to encode the other elements in the list, there is a range on the maximum length of a signature depending on the epoch or even the validator_index. To enforce more strict requirements, we propose restricting sig to length less than or equal to 934 bytes. This number assumes the other elements of the vote message take their maximal length to encode.

1024 bytes available
3 to encode the whole list
17 worst case to encode an int128
33 to encode the bytes32 hash
3 to encode the signature bytes

1024 - 3 - 17*3 - 33 - 3 == 934 bytes max for signature

Sanity checked with the following python code

import rlp
validator_index = target_epoch = source_epoch = 2**128 - 1
sig = b'\xff' * 934
target_hash = b'\xff' * 32
len(rlp.encode([validator_index, target_hash, target_epoch, source_epoch, sig])) == 1024

Note, the logout message has fewer elements so the signature could theoretically be larger than 934 for this action, but to reduce complexity, 934 should be used for logout messages as well.

Proposed Implementation

• Define MAX_SIGNATURE_LENGTH as 934
• Enforce sig as <= MAX_SIGNATURE_LENGTH in vote, slash, and logout
• tests.

[hrishikeshio]

@djrtwo Would it be better to add this enforcement in validate_signature()?

[djrtwo]

The type declaration should handle the enforcement. I was thinking we just make the type declaration the following anywhere we are referencing the sig which is a number of places at this point (including validate_signature)

sig: bytes <= MAX_SIGNATURE_LENGTH = ...

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

This issue now has a funding of 200.0 DAI (200.0 USD @ $1.0/DAI) attached to it.

• If you would like to work on this issue you can 'start work' on the Gitcoin Issue Details page.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $8,316.90 more funded OSS Work available on the Gitcoin Issue Explorer

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work has been started.

These users each claimed they can complete the work by 7 months, 2 weeks ago.
Please review their action plans below:

1) mkeen has started work.

Learn more on the Gitcoin Issue Details page.

[mkeen]

@djrtwo Your proposed solution is syntactically elegant, but from what I can see, it's going to require an enhancement to Vyper. Vyper's RLPList function sets the size of
all byte array members equal to the size of the entire RLPList.

For the purposes of decoding incoming data, Vyper's RLPList requires as arguments a bytearray, and a serial list of types -- the latter is used by RLPList to decode the former into an array of native objects. It is used like this by Casper:

casper/casper/contracts/simple_casper.v.py

Line 491 in dcf4caf

values = RLPList(vote_msg, [int128, bytes32, int128, int128, bytes])

The above doesn't expose any way to let Vyper's RLPList know how many bytes will be in the byte array members, so (again, linked above) RLPList just allocates a number of bytes equal to the size allocated for the entire message, in our case, 1024.

To summarize, due to the current implementation of RLPList in Vyper, the size of the variable sig referenced below must be equal to the size of the entire message (vote_msg) it's contained in.

casper/casper/contracts/simple_casper.v.py

Lines 486 to 496 in dcf4caf

	def vote(vote_msg: bytes <= 1024):
	# Get hash for signature, and implicitly assert that it is an RLP list
	# consisting solely of RLP elements
	msg_hash: bytes32 = extract32(raw_call(self.MSG_HASHER, vote_msg, gas=self.MSG_HASHER_GAS_LIMIT, outsize=32), 0)
	# Extract parameters
	values = RLPList(vote_msg, [int128, bytes32, int128, int128, bytes])
	validator_index: int128 = values[0]
	target_hash: bytes32 = values[1]
	target_epoch: int128 = values[2]
	source_epoch: int128 = values[3]
	sig: bytes <= 1024 = values[4]

I've looked into making some enhancements to the Vyper RLPList implementation, but it's a fairly involved undertaking, and I'd need to understand more about related project pyrlp.

Requesting a suggestion on how I should proceed.

[djrtwo]

Thanks for the deep dive! I was unaware. We might decide to truncate or something, but I am going to discuss with the formal verification team that is working with us. Leaving this open for now until we decide on a move forward.

[gitcoinbot]

@mkeen Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• warning (3 days)
• escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[mkeen]

I am still working on this. Submitted a proposal to make this possible with a change to Vyper.

[gitcoinbot]

@mkeen Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• warning (3 days)
• escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[mkeen]

@djrtwo Can I get a 10 day snooze on this? RLPList is undergoing heavy changes RN.

[djrtwo]

@mkeen I can't snooze as I'm not the founder. will have to ask @gdipri01

[djrtwo]

Cool I just saw that the VIP got approved. Congrats!

[mkeen]

Thanks!

[vs77bb]

@mkeen @djrtwo Just 10-day-snoozed Gitcoin Bot, appreciate the request and patience as we work out the kinks 🙂

[vs77bb]

Hi @mkeen just checking in, is this one still pending? Hope you are doing well 🙂

[djrtwo]

This is blocked by the implementation of a VIP vyperlang/vyper#818

It appears to be in their "final countdown" before beta. So I expect it to be finished in the relative short term

[gitcoinbot]

@mkeen Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• warning (3 days)
• escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

@mkeen due to inactivity, we have escalated this issue to Gitcoin's moderation team. Let us know if you believe this has been done in error!

• warning (3 days)
• escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[mkeen]

Hi @vs77bb yes still pending! But I am keeping an eye on it.

[vs77bb]

Awesome. Thanks for the feedback!

[kuhnchris]

Hi there everyone. Is this issue still blocked due to your dependencies?
Let me know if we can help you in any ways!

Thanks!

[hrishikeshio]

@kuhnchris The repo is deprecated. No commits since Jun 2018.

[kuhnchris]

Well, there are still 200$ in DAI stashed in the bounty from Greg. If this repository is deprecated he should cancel the bounty so he can at least get his funds back. We'll try to mail him, thanks for the info!
May I note that if this is deprecated that maybe the repository should be changed to read-only, respectively archived?

Thanks!
Chris

[mkeen]

The repo isn't deprecated as far as I know. It would be nice to at least partially claim the bounty, since I did a bunch of analysis and spec'ed out a fix upstream. Just a thought :)

[kuhnchris]

@djrtwo @hrishikeshio your calls. cc @vs77bb

[mkeen]

@djrtwo @hrishikeshio @vs77bb LMK. I think it's only fair.

[hrishikeshio]

Its deprecated according to https://github.com/ethereum/casper/blob/master/README.md.
Anyways, I am not the decision maker or bounty sponsorer. So its not my decision. good luck.

[mkeen]

Yikes.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work for 200.0 DAI (200.0 USD @ $1.0/DAI) has been submitted by:

1. @mkeen

@gdipri01 please take a look at the submitted work:

• (Link Not Provided) by @mkeen

---

• Learn more on the Gitcoin Issue Details page
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $86,212.92 more funded OSS Work available on the Gitcoin Issue Explorer