Add optional list of withdrawals to block body

[jframe]

Signed-off-by: Jason Frame jason.frame@consensys.net

PR description

Add optional list of withdrawals to block body. Also update the equals, hashcode methods. This does not wire withdrawals into the block body it's just to reduce the noise as many uses especially tests will use Optional.empty for withdrawals. Changes to wire in the list of withdrawals will be done in a subsequent PR.

Fixed Issue(s)

Documentation

• I thought about documentation and added the doc-change-required label to this PR if
  updates are required.

Changelog

• I thought about the changelog and included a changelog update if required.

[siladu]

Will this just return ", withdrawals=Optional.empty" ?

[jframe]

Yeah that's what it will return

[pinges]

LGTM

[garyschulte]

IMO overloading the BlockBody constructor would have been a better choice rather than changing all of the usages of blockbody to take an optional list...

[siladu]

IMO overloading the BlockBody constructor would have been a better choice rather than changing all of the usages of blockbody to take an optional list...

@garyschulte what's your thinking around this?
I think a benefit of doing it this way is you're potentially avoiding subtle bugs by surfacing missing wiring in the compiler.

[garyschulte]

IMO overloading the BlockBody constructor would have been a better choice rather than changing all of the usages of blockbody to take an optional list...

@garyschulte what's your thinking around this?
I think a benefit of doing it this way is you're potentially avoiding subtle bugs by surfacing missing wiring in the compiler.

Withdrawals are an Ethereum mainnet only consideration. Also now we have a bunch of Optional.empty() params everywhere, whereas an overloaded constructor could just do that in one place.

NBD, it just struck me as odd to require an optional parameter

[jframe]

IMO overloading the BlockBody constructor would have been a better choice rather than changing all of the usages of blockbody to take an optional list...

@garyschulte what's your thinking around this?
I think a benefit of doing it this way is you're potentially avoiding subtle bugs by surfacing missing wiring in the compiler.

Withdrawals are an Ethereum mainnet only consideration. Also now we have a bunch of Optional.empty() params everywhere, whereas an overloaded constructor could just do that in one place.

NBD, it just struck me as odd to require an optional parameter

You have a good point about this being mainnet specific and this forces everyone to consider whether to use withdrawals. And it would make tests cleaner. The production code though would need to determine what constructor to call so the main code may necessarily not be any cleaner. Will have a quick go-see what the code would be like using an overloaded constructor approach.