clifordsymack / Electron-Cash

Electrum; Bitcoin thin client
MIT License
6 stars 3 forks source link

Fix flat-fee model #74

Open markblundeberg opened 5 years ago

markblundeberg commented 5 years ago

If there is a desire to allow combining multiple coins (#3) then the fee model needs to be fixed first.

Result:

This also adequately covers the fees if there is a desire to move to allow multiple shuffled outputs per player.

Actual transaction size formula for reference: (10 bytes) + (148 bytes)*N_P2PKH_inputs + (34 bytes)*N_P2PKH_outputs. (148 bytes assuming longest DER ECDSA signatures; 10 bytes fixed overhead is assuming number of inputs and outputs are each less than 253)

markblundeberg commented 5 years ago

(I am raising this now since if there is a 'consensus'-breaking change, might as well include this at the same time).

cculianu commented 5 years ago

This is a very very good point you raise and I thank you for raising it.

So -- I feel tempted to go ahead, right now, before anything and do this (as part of the "use smallest input value" change in #68).

Do you think it's safe right now just to lower the fee to a hard-coded 270, and this will future-proof us for when we add additional logic to support more sophisticated shuffles?

cculianu commented 5 years ago

I'll also double-check on the P2PKH restriction. I don't think I saw it anywhere in the code. I should probably add that as a requirement (shouldn't really break anything as the client implicitly only uses such addresses right now due to wallet type). It would help to push that into the "consensus" level for blame though -- to avoid bad actors from abusing the fee structure with unusually large tx's due to larger signatures... potentially causing failures.

markblundeberg commented 5 years ago

This is a very very good point you raise and I thank you for raising it.

So -- I feel tempted to go ahead, right now, before anything and do this (as part of the "use smallest input value" change in #68).

Do you think it's safe right now just to lower the fee to a hard-coded 270, and this will future-proof us for when we add additional logic to support more sophisticated shuffles?

Well, I noticed in the code it already has some support for players bringing multiple inputs. So it looks like it could be an easy tweak.

cculianu commented 5 years ago

Well, I noticed in the code it already has some support for players bringing multiple inputs. So it looks like it could be an easy tweak

There is? Hmmm... I need to double-check that to make sure it's kosher and not a DoS vector.

Yeah so I am tempted to just lower fee to an unconditional 270 right now .. speak now or forever hold or piece if thats a bad idea... 👍

cculianu commented 5 years ago

(And when we actually allow multiple inputs, can do +200 per input...)

cculianu commented 5 years ago

For right now I added a commit that sets the fee to 270 (also bumps the protocol version number so it won't interfere with existing shufflers) in commit ba645fea8de4a9014f670d57470327034a342b2a.

I didn't add full logic to calculate fee based on number of inputs yet. Will do that later perhaps. Marking this as partially fixed.

markblundeberg commented 5 years ago

From what I can tell, once multiple inputs become actually allowed, it will be a version bump anyway. (clients won't know what to do with a peer that includes multi inputs)

cculianu commented 5 years ago

Yep. So I figure we'll cross that bridge when we come to it. 👍