ubiquity / rpc-handler

RPC Handler NPM Package
2 stars 9 forks source link

Improve trust ratio of rpcs served #60

Open Keyrxng opened 1 month ago

Keyrxng commented 1 month ago

@EresDev has made it known it's possible for pay.ubq.fi' new card feature to be taken over/hijacked by an RPC provider if it turns out they are nefarious.

const chains = await fetcher("https://chainid.network/chains.json");
const chainTvls = await fetcher("https://api.llama.fi/chains");

This is how chainlist fetch their RPC list which we also use, they validate chains based on DeFiLlama TVL stats.

Their UI produces a "Score" value but I don't think that's something we can use to determine a level of trust and I think it may be UI specific as there's nothing like that returned from in this json image

Problems:

  1. Using a service like Alchemy or Infura drastically decreases the number of networks that we can support
  2. Services like above have a service/subscription fee normally and we avoid that at all costs.
  3. I'm not aware of any other free service or service provider that fills our need here.

Solutions:

  1. general whitelist - not scalable, requires manual validation across hundreds of networks.
  2. service provider - solves the trust issues but involves costs and limits our offering.
  3. specific whitelist - cards I think are limited to our designated chains, if so, we could build a specific whitelist for those chains and only use that list for the card feature logic.

I was contacted on TG a while back with a shill for this https://github.com/erpc/erpc, which is our rpc-handler on steroids by the looks of it, idk if we are considering just replacing this package completely in place of a 3rd party offering, are we?

_original context_

0x4007 commented 4 weeks ago

My rpc manager compares permit2 bytecode to verify rpcs. It works well!

Keyrxng commented 4 weeks ago

My rpc manager compares permit2 bytecode to verify rpcs. It works well!

This is not regarding a 'health check' which is what you are referring to. Eresdev could explain better but it's that the RPC provider might be a bad actor and our RPC requests can be used to hijack the card feature.

EresDev commented 4 weeks ago

My rpc manager compares permit2 bytecode to verify rpcs. It works well!

Tell me more about it?

Solutions:

  1. general whitelist - not scalable, requires manual validation across hundreds of networks.
  2. service provider - solves the trust issues but involves costs and limits our offering.
  3. specific whitelist - cards I think are limited to our designated chains, if so, we could build a specific whitelist for those chains and only use that list for the card feature logic.

If I am not mistaken, I recall seeing "official" flag with the list of chainlist rpcs which were official rpcs by the the blockchains, but I don't see that anymore. Anyways, one of the solutions is to have such rpcs taken from official websites like https://docs.gnosischain.com/tools/RPC%20Providers/ We don't need to go for all blockchains at once. Maybe we can start with only those that we are using and later add others. This improvement can be offered by a new class RpcHandlerOfficial{} and rest of the package stays unchanged (unless you want to improve something else).

A second even easier solution is to have hardcoded trusted rpcs in the backend pages functions of the card feature, and not use rpc-handler there.

This improvement is needed only if you don't trust the rpcs by chainlist. I have mixed feelings. We can also control this by keeping a limited balance in reloadly for now.

Keyrxng commented 4 weeks ago

This improvement can be offered by a new class RpcHandlerOfficial{}

I love this approach actually because if we ever return the Ubiquity RPCs that once were then they could live here too as they'd be the most trusted

A second even easier solution is to have hardcoded trusted rpcs in the backend pages functions of the card feature, and not use rpc-handler there.

It's unclear to me whether we are supporting all networks with cards right off the bat or if you intend on releasing chains gradually, if so this sounds good but personally I'd prefer the new function as it sounds more useful and re-useable. You could pass the whitelist from the ui logic into the handler instead of constantly updating the handler pkg with new rpcs.

0x4007 commented 4 weeks ago

What exactly can a "nefarious rpc provider do"

As I understand either it can process requests accurately or revert, and nothing in between. @rndquu rfc

My rpc manager compares permit2 bytecode to verify rpcs. It works well!

Tell me more about it?

https://github.com/0x4007/ubiquity-rpc-provider/blob/2acc66088ac9eecf395d29b6d3526cce4161e012/src/rpc-provider.ts#L47

EresDev commented 4 weeks ago

What exactly can a "nefarious rpc provider do"

In our card feature backend, we read the blockchain state to confirm that the user has already transferred a permit to our treasury before giving them a card.

If the user injects the nefarious rpc (which they control), they can give answers according to our criteria, they can mint a card without transferring a real permit amount to card treasury.

0x4007 commented 4 weeks ago

If the user injects the nefarious rpc (which they control), they can give answers according to our criteria, they can mint a card without transferring a real permit amount to card treasury.

Perhaps this is possible but isn't the chances for theirs to be chosen quite slim? And they can only have one shot per permit, right? In this case we can consider it a low priority task?

rndquu commented 4 weeks ago

What exactly can a "nefarious rpc provider do"

As I understand either it can process requests accurately or revert, and nothing in between. @rndquu rfc

My rpc manager compares permit2 bytecode to verify rpcs. It works well!

Tell me more about it?

https://github.com/0x4007/ubiquity-rpc-provider/blob/2acc66088ac9eecf395d29b6d3526cce4161e012/src/rpc-provider.ts#L47

If pay.ubq.fi uses a malicious RPC node then its owner is able to generate tons of gift cards orders using fake tx hashes because here tx receipt would be retrieved exactly from a malicious RPC.

Perhaps this is possible but isn't the chances for theirs to be chosen quite slim?

Chances are slim.

I suppose the safest option is to finally run a private node and use https://github.com/ubiquity/rpc-handler as a fallback.

0x4007 commented 3 weeks ago

run a private node

This is only for mainnet. Also we couldn't get this to work again after I disabled it some time ago.

EresDev commented 3 weeks ago

I suppose the safest option is to finally run a private node

The easier option is to use handpicked official RPCs in the backend functions. 2 rpcs for each network are good enough. Easy to do and easy to maintain.

Or, this can also be automated by introducing RpcHandlerOfficial class in the rpc-handler.

Having a private node that supports all networks we use only for this purpose feels like more work and probably some cost.

rndquu commented 3 weeks ago

I suppose the safest option is to finally run a private node

The easier option is to use handpicked official RPCs in the backend functions. 2 rpcs for each network are good enough. Easy to do and easy to maintain.

Or, this can also be automated by introducing RpcHandlerOfficial class in the rpc-handler.

Having a private node that supports all networks we use only for this purpose feels like more work and probably some cost.

Whitelisting a public "official" node or even a private node (deployed via smth like https://chainstack.com/protocols/gnosis-chain/) doesn't solve the issue when RPC node becomes malicious.

Here are 2 issues actually:

  1. We need a list of stable RPC nodes (which is some cases not the fastest RPC node in a list from https://chainlist.org/). So perhaps it makes sense (as @EresDev proposed) to have a new class RpcHandlerOfficial which would offer a list of 2-3 stable RPCs. It will be our job to manually maintain that list.
  2. On the pay.ubq.fi backend side we can't really trust any RPC node (unless it's a private RPC node hosted on our side). We could introduce a simple consensus check, like if tx receipt exists at least in 2 RPC nodes then there's nothing malicious. This is kind of a overoptimization looking at the current "mint gift card" feature state but it'd good not to forget the issue exists.
0x4007 commented 3 weeks ago

I like 2 better because then we don't have to worry about this problem again.

I think it could be interesting to add a special method to the RPC handler where it references something like _trustedCall(payload, minimumConsensus?) and then will throw unless minimum consensus has been achieved.

We can use this method exactly for applications like this.

But it isn't clear to me how we can make the request to test the response without writing to the chain.