NCC-2016-016 - Generous Miners using Alternative Clients may Cause Forks

[rcseacord]

We're currently assessing impact before publishing more detail.

[daira]

The Zcash protocol specifies that a founder’s reward will be paid out to a specific address up until a certain point in time.
Currently, there are two ways to pay into this founder’s address:

1. As the founder’s address is a public, normal address, any user can simply create a
   transaction with the founder’s address listed as the output.
2. Each block that is mined must send 20% of the reward to the founder’s address.
   When checking the validity of a block, the 20% of a reward is checked at location
   main.cpp:3075: [...]
   The straight equality check may lead to forks if a miner is particularly generous. If an alternative client is put forth which allows more than 20% of the reward being sent to the founder address, forks may arise because of the difference in consensus rules.

Recommendation
Instead of a strict equality check, perform a greater-than-or-equals check against 20% of the block subsidy.

[daira]

This is specified as intended. I will clarify the protocol specification to make the strict equality a MUST requirement, ensuring that this is clear to implementors of alternative clients. A miner (or anyone else) may donate any additional ZEC to Founders' addresses if it feels like it, but must do so in a separate output. There is a potential spec ambiguity in the case of multiple outputs to the correct Founders' address, which I will also clarify and check against the implementation.

[daira]

Fixed in version 2016.0-beta-1.7 of the protocol spec. I verified that the consensus enforcement code matches the new spec (sections 6.5 and 6.6), but I'd appreciate it if someone double-checked that before closing this ticket.

[str4d]

In the engineering meeting, either @bitcartel or @ageis said they would double-check the code against the spec. I can't remember which.

[bitcartel]

1. The testnet addresses in the spec need to be updated with the latest version in chainparams.cpp.
2. The formulas in the spec for FounderAddressChangeInterval and FounderAddressIndex are visually different from the code, making use of ceiling and floor. Could that confuse people?

[daira]

[@bitcartel's comment confuses me because it seems to be for #1477, but I answered it here anyway.]

I've just done 1. (in 2016.0-beta-1.8).

For 2., we could add a comment, but I don't see that as a blocker for this release.

[str4d]

I'm satisfied that the spec and code are consistent. ACK.