-
Notifications
You must be signed in to change notification settings - Fork 220
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
catalog storage costs of smart-wallet operations #5985
Comments
I'm inclined to re-phrase this as a bug:
I don't yet have evidence that it is vulnerable, but I think the premise of this issue is that we presume that it is. |
Is this truly a pso blocker if we set Gas sufficiently high? |
we don't know. That is: it's a blocker until I/we do at least a little info gathering. |
Smart Wallet Exposed API
What's currently on master is:
The parameter to each is a string that gets JSON.parse'd and then
I don't have a good feel for exactly what happens in step 4. Tracking PaymentsI'm not exactly sure about the flow here. I know that there was a problem where payment records grew with each update (fixed in a pending PR). no petname control
That's not an option in the current API. |
Somewhat careful study of the walletFactory (aka smartWallet) contract shows that storage and compute costs are accounted for. I suppose the next step is to turn this over to the kernel team to measure experimentally. @Tartuffo I'm routing this thru you. cc @turadg walletFactory: provideSmartWalletWe charge 10 BLD for this, and it does O(1) work and allocation (provided smartWallet.fromBridgeWe charge We validate against this shape:
It does return a return promise from smartWallet.fromBridge: drop?The bootstrap vat drops the promise, so perhaps we should drop the promise inside the walletFactory vat and return void rather than returning the promise? void E(srcHandlers.get(srcID)).fromBridge(srcID, obj); wallet.handleBridgeActionwe offers.executeOfferguarded as I don't see any loops in the code paths of chainStorage usage: about 6 x 400bytes per PSM tradeAs an offer progresses (a seat allocated, offerResult available, wants satisfied) the status is published, along with give / want / payouts records, to chainStorage. It seems to be about 6 updates per PSM trade, each of 400 bytes or less: #6038 (comment) Continuing offers (for gov cttee voting) accumulate indefinitelyThis only applies to voting, not to PSM trades. And only gov cttee members have invitations that give them access to start the voting process. vbank asset notifierEach time an asset is added the vbank, a notification goes out to all wallets, and they all add a purse. The power to add vbank assets is reserved to the BLDer DAO. |
return promise from smartWallet.fromBridge: drop?The bridge vat drops the promise, so perhaps we should drop the promise inside the walletFactory vat and return void rather than paying for an inter-vat promise? agoric-sdk/packages/vats/src/bridge.js Line 49 in a62c1f8
cc @turadg |
TIL: returning void won't save the inter-vat promise. To do that requires something more like |
This is satisfactory for PSM-First. |
What is the Problem Being Solved?
In today's Wallet meeting, we were talking about storage-consumption attacks against the smart wallet vat. Each provisioned smart-wallet account can send signed instruction messages (via the bridge device) into this vat, and the code there will interpret those instructions to do things like initiate transfers, assign petnames, and create/accept offers. The smart wallet retains some amount of data for each account: purse/payment references, petname assignments, open offers, etc. (We keep these on the chain side specifically so that the user doesn't need to remember anything more than their seed phrase).
We'd like to know how much storage space each one of these options might incur, to reason about how much it would cost an attacker to try and exhaust this storage. Once the smart wallet (and the vats it depends upon) gets fully virtualized (#4489), we don't think RAM usage should be a problem, but disk/DB usage is still a consideration. For example, each user has a table of petnames that map strings to/from object references like Issuers. This is probably stored as a pair of BigDurableMapStores, with string keys (and Presence/vref values) and vice versa. Adding one entry to a virtual/durable store like this costs one entry in the LMDB database, whose DB key is a serialized representation of the string key (this is cheap, we merely prefix an
s
), and whose value is the capdata serialization of the target's vref (something like{"body":"{\\"@qclass\\":\\"slot\\",\\"iface\\":\\"Alleged: setStore\\",\\"index\\":0}","slots":["o+8/11"]}
). LMDB has some overhead, but the data we feed into the DB for a petname of length L would then be about1+L+96
.If a smart-wallet user chooses to store really large petnames, or a lot of them, they could drive the smart-wallet vat to use a lot of DB space, which then adds to the disk requirements on all validators and followers. In the long run, we want the cost of this to be captured in a meter and charged to the user. But in the short term, we just need to make sure this can't cause problems.
The task is to
I can think of two ways to measure this:
.agoric/data/ag-cosmos-chain-state/data.mdb
) as we submit hundreds or maybe thousands of the same operation, to measure the slope of DB size vs num_operationsdata.mdb
file might be sparse and pre-allocated to be pretty large, so might need to look at the actual used blocks (not just whatls -l
reports), or e.g. compress it and use the compressed size as a proxy for the actual consumptionkey.length + value.length
and accumulate it across all add/modify/delete DB operationsLMDB might have some sort of API to ask about total used space too, which would be the best option. I see http://www.lmdb.tech/doc/group__mdb.html#structMDB__stat , maybe the "pages" it counts are used for both keys and values. It might be good to enhance
@agoric/swing-store
to add agetStats()
method, and to include this number in the telemetry metrics we report out of the chain (and into tools like honeycomb).In either case, we'd need to build a test scaffold that can submit N copies of the operation under test, and measure the DB size every once in a while, and build up a graph to correlate them.
@dckc is going to enumerate the smart-wallet operations and the user-controlled parameters of eah, so we can figure out what our experiment needs to test.
Description of the Design
Security Considerations
Test Plan
The text was updated successfully, but these errors were encountered: