near / nearcore

Reference client for NEAR Protocol
https://near.org
GNU General Public License v3.0
2.33k stars 627 forks source link

New gas cost for zero balance accounts #8427

Closed jakmeier closed 1 year ago

jakmeier commented 1 year ago

To support zero balance account (https://github.com/near/NEPs/pull/448) we need to check the effect of increasing gas costs for account creation on existing contracts.

The rough idea is that we first list all contracts that have used account creation in the past and then we check which of those would have failed with the new cost.

jakmeier commented 1 year ago

Progress update: I have extended #8371 to also list all actions used by these accounts. It requires iterating all receipts in an archival node, which can be slow but should be doable when iterating in RocksDB order.

Listing all actions for all accounts in shard 1 took 4h, which seemed reasonable, But even after 30h, the same process has not finished yet for all shards combined. I don't really have a way to check how much of the total progress has been done.

To optimize this, there are several ideas:

I will work on some of these optimizations but still hope that the running process will finish faster than my optimizations are ready.

jakmeier commented 1 year ago

I had to restart and run with improved code, as the old version would have never pushed through shard 2 and 4. The new run sits at a bit above 150M transactions processed out of ~300M. It is expected to finish in 12-14h from now. I will update once I had a chance to look at the results.

jakmeier commented 1 year ago

I miscounted my progress estimate, I thought there are only 300M receipts because there are that many transactions. Don't know how many receipts per average there are per tx, but I would assume less than 2? FT transfers are just one receipt, all of Sweat's record_batch also only have one receipt. I would think it's a minority that has more than 1.

It's now sitting at 421M receipts processed and still going up at ~12M receipts per hour. Can't be much longer now...

Edit 1: [2023-01-26T19:49:21Z] 485M and counting Edit 2: [2023-01-27T10:01:52Z] 617M ... Edit 3: I forgot about refund receipts! That's why there are so many receipts...

jakmeier commented 1 year ago

It finally finished after processing 681400000 receipts. Even while filtering out some of the largest contracts, it still took a while to finish.

real    4391m49.315s
user    1428m12.224s
sys     589m7.898s

There are 34 out of 24213 accounts that have (ever) created an account from a function call. Here is the full list:

ACCOUNT_ID RCPTS_IN RCPTS_OUT ACTIONS
8o8.near 43 23 CreateAccount,DeployContract,FunctionCall,Transfer
account.bhc8521.near 5 5 CreateAccount,Transfer,AddKey
app10.hipodev.near 748 1029 CreateAccount,DeployContract,FunctionCall,Transfer,AddKey
cdao.near 35 49 CreateAccount,DeployContract,Transfer,DeleteAccount
cfac.mapprotocol.near 3 5 CreateAccount,DeployContract,FunctionCall,Transfer
contract.portalbridge.near 4637 7381 CreateAccount,DeployContract,FunctionCall,Transfer,AddKey
daofactory.near 305 181 CreateAccount,DeployContract,FunctionCall,Transfer
drawstring_v1.near 12 8 CreateAccount,DeployContract,Transfer,AddKey
drawstring_v2.near 1273 566 CreateAccount,DeployContract,FunctionCall,Transfer,AddKey
drawstring_v3.near 112 112 CreateAccount,DeployContract,FunctionCall,Transfer,AddKey
embr.playember_reserve.near 1346452 933559 CreateAccount,FunctionCall,Transfer,AddKey,DeleteKey
factory.bridge.near 158740 244425 CreateAccount,DeployContract,FunctionCall,Transfer,AddKey
factory.pulsemarkets.near 17 20 CreateAccount,DeployContract,FunctionCall,Transfer
factory_guildx.near 23 23 CreateAccount,DeployContract,FunctionCall,Transfer
laboratory.jumpfinance.near 11 12 CreateAccount,DeployContract,FunctionCall,Transfer
linkdropv1.near 1107 1542 CreateAccount,DeployContract,FunctionCall,Transfer,AddKey
lockup.near 5937 1196 CreateAccount,DeployContract,FunctionCall,Transfer
lockup.pierre-dev.near 12 19 CreateAccount,DeployContract,FunctionCall,Transfer
mfac.butternetwork.near 3 5 CreateAccount,DeployContract,FunctionCall,Transfer
mfac.mapprotocol.near 4 5 CreateAccount,DeployContract,FunctionCall,Transfer
mintbase1.near 5803 8697 CreateAccount,DeployContract,FunctionCall,Transfer,AddKey
multisafe.near 48 89 CreateAccount,DeployContract,FunctionCall,Transfer
near 15565659 20562046 CreateAccount,FunctionCall,Transfer,AddKey,DeleteKey
newtoken.near 53 60 CreateAccount,DeployContract,FunctionCall,Transfer
nft.radianceteam.near 3 5 CreateAccount,DeployContract,Transfer,AddKey
nftsale.near 7 7 CreateAccount,DeployContract,FunctionCall,Transfer
non-staking-lockup.near 18 25 CreateAccount,DeployContract,FunctionCall,Transfer
pack_minter.playible.near 582 733 CreateAccount,DeployContract,FunctionCall,Transfer,AddKey
pool.near 3845 104 CreateAccount,DeployContract,FunctionCall,Transfer
poolv1.near 1407 2365 CreateAccount,DeployContract,FunctionCall,Transfer
sputnik-dao.near 3787 5423 CreateAccount,DeployContract,FunctionCall,Transfer,AddKey
sputnikdao.near 347 632 CreateAccount,DeployContract,FunctionCall,Transfer,AddKey
the-auction-factory.near 37 43 CreateAccount,DeployContract,FunctionCall,Transfer
tkn.near 916 1370 CreateAccount,DeployContract,FunctionCall,Transfer
bowenwang1996 commented 1 year ago

@jakmeier thanks for the information! We should definitely check the ones with high usage such as near, lockup.near, sputnik-dao.near, pool.near, poolv1.near to see if they would be affected

jakmeier commented 1 year ago

I've looked at single receipts of some of the listed accounts and so far I couldn't find any instances where increasing the gas cost by 5.8 Tgas would cause any problems. For example create_account in near is usually called with plenty of extra gas and without extra indirection.

Sadly, I noticed a problem in my optimized code which caused only accounts < token.sweat to be analyzed... So we are missing accounts starting with u-z.

The good news: A fixed script has been running all weekend. And I also extended the output with some trickery around refund receipts which will give us the gas amount for each account of how much we could increase the CreateAction cost without breakage. The query is at 55% now and is expected to finish in about 48h from now. Once it concludes, assuming no further errors, we will have our final answer.

jakmeier commented 1 year ago

I have the full data now. 7 accounts more were found after fixing the bug. And I now also have the minimum refund for each contract after they created an account. See the large table below.

Click me to expand full table Account ID | Minimum Refund [Tgas] -- | -- embr.playember_reserve.near | 1.208208 near | 4.28617 drawstring_v2.near | 4.368422 the-auction-factory.near | 9.436245 contract.portalbridge.near | 10.996162 sputnikdao.near | 13.901099 drawstring_v1.near | 22.405993 multisafe.near | 26.056609 lockup.near | 26.217698 factory.bridge.near | 26.218371 account.bhc8521.near | 27.217027 lockup.pierre-dev.near | 28.158412 user_settings_v1.prefs.near | 29.278897 v2.nearp2pdex.near | 37.451086 v1.multicall.near | 39.989198 v1_03.multicall.near | 40.01233 8o8.near | 42.941968 sputnik-dao.near | 46.954982 linkdropv1.near | 63.352007 nftsale.near | 73.7272 laboratory.jumpfinance.near | 75.457096 mintbase1.near | 86.905321 wentokensir.near | 87.731546 factory.pulsemarkets.near | 92.085501 non-staking-lockup.near | 122.77716 newtoken.near | 131.586182 tkn.near | 131.94001 mfac.butternetwork.near | 137.312404 drawstring_v3.near | 142.217183 app10.hipodev.near | 146.602622 v1.cellfi-prod.near | 147.517368 cfac.mapprotocol.near | 172.425073 pool.near | 172.939378 mfac.mapprotocol.near | 179.81763 cdao.near | 193.233035 poolv1.near | 197.756252 factory_guildx.near | 209.028035 daofactory.near | 240.711197 pack_minter.playible.near | 270.382218 nft.radianceteam.near | 276.522799 x-to-earn.near | 298.181344

Three contracts have seen traffic in the past that would fail with the planned cost increase of 6Tgas.

Account ID Minimum Refund [Tgas] example receipt analysis
embr.playember_reserve.near 1.208208 [6oXqCkC5N7pYKGEv59j9vp 8YAupf5wy7vMAJ4jcEF4Ns]() sender can attach more gas off-chain
drawstring_v2.near 4.368422 [AAkUfRyPJvaHvpfkAvi7USP SAZud61qjnD1cCs2CfRUn]() sender can attach more gas off-chain, also drawstring_v3.near already exists and has always had >100Tgas to spare
near 4.28617 CKZDYuJBMxp8NugVZ5ZMtn g9sPn5tGbXXQhcGrtDRaBd *see below

* The registrar account near is tricky. Normal usage is to call it directly and this usecase is certainly not breaking. However, there are at least some contract that have called it with only ~4Tgas to spare.

@bowenwang1996 If we increase the costs as planned to 6 Tgas per account creation, it can potentially break some existing code calling into near::create_account. If we set the cost to 4 Tgas instead, it will be fine for sure.

Also, I think 6Tgas would be okay if we make a small adjustment to the code in near, attaching less to the callback which currently uses too much gas. Do you know where that code is on Github and who would be in charge of updating it?

I don't have the full list of accounts that could be affected, only the 10 worst receipts. nearcon.keypom.near and several instances of linkdrop (e.g. linkdrop.theoutsiders.near)) are the only confirmed accounts to break in that list.

Similar patterns could be used in other contracts. It would take more coding work and more time to find all of them.

Analysis of a breaking call to near::create_account

The example receipt above is from linkdrop.theoutsiders.near::create_account_and_claim() cross-contract calling near::create_account and waiting for the callback. They start with 100 Tgas but only send 30Tgas to near::create_account. The other 70 Tgas are spent on linkdrop.theoutsiders.near::on_claim and theoutsiders.near::nft_transfer. But that's not relevant.

Inside near::create_account, ~6 Tgas is burnt on the call itself, ~20 Tgas used for near::on_account_created and ~4Tags are refunded. That's why we have only ~4 Tgas to spare in this case, or account creation would fail.

Of the ~20Tgas attached to near::on_account_created, only ~3 Tgas is actually burnt, the remainder is refunded. So the solution in this case would be to attach less to near::on_account_created.

bowenwang1996 commented 1 year ago

Thanks @jakmeier for the detailed analysis

@bowenwang1996 If we increase the costs as planned to 6 Tgas per account creation, it can potentially break some existing code calling into near::create_account. If we set the cost to 4 Tgas instead, it will be fine for sure.

The code can be found in this repository

They start with 100 Tgas but only send 30Tgas to near::create_account.

Is 100Tgas modifiable? We have the feature where we would split unused gas evenly to each cross contract call, so I wonder whether it could be fixed that way.

jakmeier commented 1 year ago

@bowen Not linkdrop, I need the registrat account :)

100Tgas can be modified yes,it's set off-chain

bowenwang1996 commented 1 year ago

@bowen Not linkdrop, I need the registrat account :)

Sorry I am confused. I think linkdrop is the contract that is deployed on near

jakmeier commented 1 year ago

@bowen Not linkdrop, I need the registrat account :)

Sorry I am confused. I think linkdrop is the contract that is deployed on near

Oh, really? I didn't know / expect that. Sorry then I got confused,

In that case, I think we could lower ON_CREATE_ACCOUNT_CALLBACK_GAS from 20 Tgas to 15 Tgas. That would compensate for the increased cost of the scheduled create account promise.

Or better, as you suggested, make use of the new-ish function_call_weight. I think allocating all of the unused gas to the function call works best here (sertting gas=0). At least that would make the most sense to me. But I suppose the maintainers of that contract should decide.

@bowenwang1996 I think we have a clear answer: Only the linkdrop account needs an update, and it shouldn't be a difficult change. Do you need anything more from my side?

bowenwang1996 commented 1 year ago

@bowenwang1996 I think we have a clear answer: Only the linkdrop account needs an update, and it shouldn't be a difficult change. Do you need anything more from my side?

We are good for now. Thanks for the help!