Skip to content
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

Dynamic fees V1 #1

Merged
merged 11 commits into from
May 5, 2021
Merged

Dynamic fees V1 #1

merged 11 commits into from
May 5, 2021

Conversation

jcape
Copy link
Contributor

@jcape jcape commented Apr 26, 2021

A simple proposal for start-time configurable fees.

Rendered RFC

@jcape jcape self-assigned this Apr 26, 2021
@jcape jcape requested a review from a team April 26, 2021 21:44
James Cape and others added 3 commits April 28, 2021 12:53
Co-authored-by: sugargoat <sara.drakeley@mobilecoin.com>
Co-authored-by: sugargoat <sara.drakeley@mobilecoin.com>
Co-authored-by: sugargoat <sara.drakeley@mobilecoin.com>
James Cape added 2 commits April 28, 2021 17:07
Note that following enclave-upgrade restart procedures will prevent pending transactions from being lost.

Choosing to simply accept that if any node has accepted a proposed transaction at a lower fee, that fee is (or would have been) valid for all nodes is an option, but this does make the ability to enforce fees dependent on the union of all SGX functionality, which is a change from our ad-hoc norms around the use of SGX as a privacy-enhancement technology, rather than a security-critical technology.

The second option is to only check fees lazily, which will produce the scenario where a client could submit a transaction, and have it appear to "time out" later, because it was accepted into the queue at a low fee, then later judged to have an insufficient fee.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice:

  1. there should be an expectation in the ecosystem about how much the 'lazy' minimum fee can vary over short timespans
  2. the default fee used by clients can be some multiple of the minimum fee, above the potential variance over e.g. 100 blocks (current tombstone limit)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

actually, both of those points apply to any solution, lazy or otherwise

@jcape
Copy link
Contributor Author

jcape commented Apr 29, 2021

Chat transcript:

@mfaulk 11:46

The dynamic fees RFC looks solid to me, although it would be nice to not have to restart nodes in order to adjust the fee. Here's a simple suggestion that might help with that:
The enclave only enforces fee constraints that don't need to be adjusted, e.g. "fee > 0".
Each node maintains a minimum_fee parameter.
When a client submits a transaction to a node, that node rejects the transaction if its fee is less than minimum_fee. Otherwise, it processes the transaction as normal.
The idea here is that each node is a "gatekeeper" that enforces a fee minimum before a transaction gets into the consensus mechanism, but that once a transaction is in consensus, the fee value isn't important. In general, node operators will coordinate updating fees together, but nodes with different minimum_fee values would be able to coexist on the network together (which is nice for things like performing rolling updates). Since each node's minimum_fee value is not part of its enclave, it can be changed without restarting the node.
The obvious downside to this is that a single node could set their minimum_fee very low, but that might not actually be a problem. That node would still have to validate every incoming transaction and propose the transactions for nomination, so the impact of this would be a fraction of transactions with unusually low fees getting accepted by consensus. This behavior would be visible to all nodes, and so could be handled at the "hey, your node is misconfigured" level.

@jcape 12:41

I generally agree with that, but you could get to the same place by simply letting each node's minimum fee be independently configured.

12:41

i.e. not do kex and let SCP tx validation handle it

12:42

Like, my understanding of the transaction handling is that we do the is_well_formed() (read: is_valid()) test on all transactions we receive from clients, before we put them into the tx cache destined for SCP, and also do that on all tx received from peers.

12:43

And the validation test includes the minimum fee.

12:44

So just adjusting a particular node's fee and not telling anyone would have the same effect.

12:46

Though there is one difference: an "authenticated" node (e.g. signal's node), lets you have a low fee for trusted clients, whereas unrestricted nodes can continue to charge high fees

@UkoeHB 12:47

there is a difference between ‘ignoring a tx from client’ and ‘a tx is invalid’ - if you ignore a tx from client, you may still be ‘willing’ to accept that tx when a peer recommends it

@jcape 12:47

Yes

@UkoeHB 12:47

I think @mfaulk is recommending ‘ignore tx from client’

james 12:47

Yes, he is (or that's my read of it). (edited)

koe 12:49

let SCP tx validation handle it

ah, this looked to me like adding a locally-defined ‘tx validity’ rule, which can break SCP (edited)

@jcape 12:50

Yes, that's my mis-reading of it.
Implementing this directly means moving the fee validation out of mc_transaction_core::validation::validate() and into client_tx_propose() instead.

12:51

There is a "race to the bottom" potential by excluding it from kex, though.

@UkoeHB 12:51

race to the bottom?

@jcape 12:52

e.g. [exchange] has a commercial incentive to keep their fees near zero

@UkoeHB 12:52

oh I see, yeah it allows a perverse incentive (edited)

@jcape 12:53

I suppose that is sorted in the usual "OK, GTFO, bro" if a node operator is behaving badly.

@UkoeHB 12:54

in terms of ‘short term solutions’, it may be simpler to implement

@jcape 12:54

i.e. if someone leaves their fees low, and their node is a source of trash, other operators can just remove them

12:57

Well, the RFC as-written is already mostly implemented (but not merged), changing it to this new version is pretty trivial, though---it's removing the hacks around adding the fee to the ResponderId, and moving the fee check to happen in a different place than it does currently.

...and explain why we should move forward without it for now.
Copy link

@jayzalowitz jayzalowitz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thoughts on cloud options wrt preventing any effective ddos


The "restart for each change" system will have similar properties for users as an emergency restart to introduce new fees---when a node is restarted without following the upgrade procedure, any pending transactions it has are lost---but is dramatically simpler to implement, and does not require special thought about validity periods of a particular transactions' fee-paid status.

## Alt: Allow Divergent Fees
Copy link

@jayzalowitz jayzalowitz Apr 30, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Divergent fee makes a lot of sense to me. For example I may be willing to conduct transaction processing when spot instances come up for sale on aws/azure/etc but not when that is not the case. This allows me to operate at 10% of the cost. I can leave a "pilot light" up at higher costs and then spin up unlimited clusters of more expensive software when that happens.
This makes the network almost impossible to ddos as you'd have to be able to take down aws, azure, and gcloud simultaneously, and at that point why waste it on mob

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My instincts prefer divergent fees as well, but there's a bunch of gotchas that we'd have to talk our way through in order to get it to a place that feels comfortable---IMO it's a great candidate for the next "dynamic fees" RFC, where a fuller discussion is possible without the "OMG, tx costs $0.50" overhang :-)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So I can speak as a global expert when I say the following: Unless you standardize deployment in such a way that all the node operators have a couple of unneeded SPOF, everybody's infra is gonna run at a different cost, and whats more, at a certain repayment level, we can easily handle more load, its pretty much a demand economy in cloud nowadays.

(I will personally audit and make recommendations to node operators to run cheaper if you @ me on socials)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If a tx costs 50c to run, yall arent running all of the nodes in an efficient way. The aws minimum cost for an entire hour of time on a server that can run a (virtualized) SGX is 0.192000 ... are we really running one tx every 2.5 hours per node... seems unlikely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "$0.50" refers to 0.01MOB fee at $50/MOB. Whatever the actual costs to validating node operators, it costs users $0.50 (well, $0.325 as of this writing) to conduct a transaction.

@jcape jcape added A-fees Fee-related proposals T-consensus Consensus Team T-sdk Mobile SDK Team final-comment-period This RFC is in the final-comment period for the next 24h labels May 3, 2021
@jcape
Copy link
Contributor Author

jcape commented May 3, 2021

This has approval from consensus and the sdk and broad agreement, going to merge the RFC in 24h unless there's a big show-stopper issue in that time.

@jcape jcape merged commit a1afb64 into mobilecoinfoundation:main May 5, 2021
@jcape jcape deleted the dynamic-fees-v1 branch May 5, 2021 17:29
@jcape jcape removed the final-comment-period This RFC is in the final-comment period for the next 24h label May 5, 2021
jcape pushed a commit to mobilecoinfoundation/mobilecoin that referenced this pull request May 7, 2021
* Implementation of mobilecoinfoundation/mcips#1 in consensus
* Add dynamic fee support to mc-connections, mobilecoind, and slam.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-fees Fee-related proposals T-consensus Consensus Team T-sdk Mobile SDK Team
Projects
None yet
7 participants