Skip to content

Conversation

@darwinzer0
Copy link
Contributor

This PR adds a new API function callable within a contract that will evaporate (burn) a specified amount of SDK gas. The purpose of this API function is to allow contracts to mask information about which message type was executed by observing the gas_used.

In practice, clients that choose to opt-in would know in advance how much actual gas an execution will incur, so they set an evaporation field in their execution message that specifies exactly how much extra gas to consume for an individual execution. For example, with SNIP-2x, almost every message type emits a distinct gas_used, so the client might choose a precise evaporation parameter for each message in order to effectively pad the gas_used of the execution so that it ends up being the same and thus indistinguishable for all message types (and in such a way that is guaranteed to happen within the enclave).

In addition to this PR, I will do one for the cosmwasm repo as well.

A simple example contract using the API function can be found here: https://github.com/SolarRepublic/evaporation-test. Supplementary to this, we would also produce a new specification, e.g., SNIP-50, that describes the interface for evaporation, such that any contract (token, NFT, or otherwise) could support in order to take advantage of this privacy-preserving feature; as well as a reference implementation.

@darwinzer0 darwinzer0 changed the title New gas_evapoate API function New gas_evaporate API function Mar 19, 2023
@assafmo
Copy link
Contributor

assafmo commented Mar 27, 2023

That's freaking awesome

/// query export, which queries the state of the contract.
fn query_chain(request: u32) -> u32;

/// Evaporates a specified amount of sdk gas (where 1000 cosmwasm gas : 1 sdk gas)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd abstract away "cosmwasm gas" and just do 1 evaporate = 1 sdk gas

Copy link
Contributor

@assafmo assafmo left a comment

Choose a reason for hiding this comment

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

Awesome idea overall. Please add Go tests

@assafmo
Copy link
Contributor

assafmo commented Mar 27, 2023

Copy link
Member

@Cashmaney Cashmaney left a comment

Choose a reason for hiding this comment

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

LGTM (agree with Assaf's comment)

The one question I have is how would a contract know how much gas to evaporate during runtime? We'd need to add another api to query the current usage (I don't think we have one right now?)

@assafmo
Copy link
Contributor

assafmo commented Mar 27, 2023

I think maybe a better solution might be to abstract this API function using primitives in secret-toolkit, e.g. a Uint128 that randomly pads the value with 1-10 bytes before encryption and seamlessly abstracts that from the dev when reading from storage.

@Cashmaney
Copy link
Member

I think maybe a better solution might be to abstract this API function using primitives in secret-toolkit, e.g. a Uint128 that randomly pads the value with 1-10 bytes before encryption and seamlessly abstracts that from the dev when reading from storage.

That's waaaay more complex and really hard to do in a way that doesn't leak information. It's a possible improvement and not mutually exclusive with this solution

@darwinzer0
Copy link
Contributor Author

The one question I have is how would a contract know how much gas to evaporate during runtime? We'd need to add another api to query the current usage (I don't think we have one right now?)

I thought about this but wasn't sure if current gas usage is deterministic. Is there a chance you could change the state based on the return value and break consensus?

Also, is current usage based on all the messages that might have been broadcast prior to the current tx? If so, that means you don't really know what your target gas should be from inside a given contract. There's no way to specify how much gas you want to allocate to each tx in a broadcast, right?

One practical approach is to add an evaporate field to all the messages (like padding) and let the client set the value. Then based on historical gas usage wallets could set evaporate for different message types (eg for snip20). This is also flexible because if gas metering changes with an upgrade, then you just change the field value in new messages.

@Cashmaney
Copy link
Member

The one question I have is how would a contract know how much gas to evaporate during runtime? We'd need to add another api to query the current usage (I don't think we have one right now?)

I thought about this but wasn't sure if current gas usage is deterministic. Is there a chance you could change the state based on the return value and break consensus?

Also, is current usage based on all the messages that might have been broadcast prior to the current tx? If so, that means you don't really know what your target gas should be from inside a given contract. There's no way to specify how much gas you want to allocate to each tx in a broadcast, right?

One practical approach is to add an evaporate field to all the messages (like padding) and let the client set the value. Then based on historical gas usage wallets could set evaporate for different message types (eg for snip20). This is also flexible because if gas metering changes with an upgrade, then you just change the field value in new messages.

It kind of depends. For the most part contracts tend to use a fairly similar amount of gas each usage. The exact amount often changes depending on the state, but it should be bounded in some range. If the contract is using arrays or something that can grow forever and get saved to state that's a whole other issue.

I think we should add an api to allow contracts to query current usage, and then contracts can implement logic that gives them flexibility on how much gas to consume at the end of a run - they can aim for a fixed amount, round up, or allow the client to provide an input. What do you think?

instance: &wasm3::Instance<Context>,
evaporate: i32,
) -> WasmEngineResult<i32> {
let evaporate = (evaporate as u32) as u64 * GAS_MULTIPLIER;
Copy link
Member

Choose a reason for hiding this comment

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

Oh, btw this needs a check if evaporate is 0 and if not to evaporate some base gas (specified in gas_costs) - otherwise you can DoS the chain by just calling this endlessly with 0 gas evaporated

@blake-regalia
Copy link
Contributor

I think we should add an api to allow contracts to query current usage, and then contracts can implement logic that gives them flexibility on how much gas to consume at the end of a run - they can aim for a fixed amount, round up, or allow the client to provide an input. What do you think?

If the developer can query for current usage, that would indeed allow for the best results. It shifts the onus to contract devs, but it solves the "greedy" contract problem where one contract over-evaporates gas and doesn't leave enough for subsequent callbacks/executions.

For example, if the executed method does not make any calls to other contracts, then the dev can safely evaporate 'all' remaining gas, vs. if the method is invoking say a receiver callback, then the dev can evaporate up to some fixed target amount (say 50k GAS) while reserving whatever remainder for downstream executions.

In this approach, I wouldn't see the point in allowing the client to specify some arbitrary amount to evaporate anymore.

@darwinzer0
Copy link
Contributor Author

I think we should add an api to allow contracts to query current usage, and then contracts can implement logic that gives them flexibility on how much gas to consume at the end of a run - they can aim for a fixed amount, round up, or allow the client to provide an input. What do you think?

If the developer can query for current usage, that would indeed allow for the best results. It shifts the onus to contract devs, but it solves the "greedy" contract problem where one contract over-evaporates gas and doesn't leave enough for subsequent callbacks/executions.

For example, if the executed method does not make any calls to other contracts, then the dev can safely evaporate 'all' remaining gas, vs. if the method is invoking say a receiver callback, then the dev can evaporate up to some fixed target amount (say 50k GAS) while reserving whatever remainder for downstream executions.

In this approach, I wouldn't see the point in allowing the client to specify some arbitrary amount to evaporate anymore.

This works except for cases where an error is thrown and execution ends before the evaporate API function is called. That's why I called the evaporate function at the beginning of the execute function in the sample contract. But practically speaking it might not matter if errors are different (you would only maybe see what function the user was trying to execute).

@blake-regalia
Copy link
Contributor

In this approach, I wouldn't see the point in allowing the client to specify some arbitrary amount to evaporate anymore.

On second thought, multi-msg txs complicate this since they can include multiple, independent contract executions that share the same pool of gas, in which case an individual contract should never consume 'all' the remaining gas. This was another reason we initially designed for client-specified evaporation, but it might be easier to just say that multi-msg txs are simply out of scope and not eligible for evaporation..?

@assafmo
Copy link
Contributor

assafmo commented Mar 29, 2023

I thought about this but wasn't sure if current gas usage is deterministic. Is there a chance you could change the state based on the return value and break consensus?

It's deterministic, you can't break consensus by knowing your current gas usage.

an individual contract should never consume 'all' the remaining gas

I disagree. There's no point in trying to mitigate this because a contract can consume all remaining gas anyway using an endless loop. IMO there's no need to treat multi-msg txs differently here.

If the developer can query for current usage, that would indeed allow for the best results.

When a contract is executed, its gas limit is the remaining gas in the tx. So the query gas API that you're talking about here can only return 3 things:

  1. The gas limit of the current contract call
  2. The gas used by the current WASM VM execution
  3. The external gas used by the current execution (storage access)

Though it's best to just combine 2 & 3.

@Cashmaney
Copy link
Member

Thanks for the changes. We'll probably add a default base gas that is consumed by calling evaporate instead of going the route of defaulting to 1 if 0 (1 gas is almost nothing, so just the host call probably consumes more relative CPU time), but at this point I think we can just merge this and tinker with it after? Any other comments @assafmo?

@darwinzer0
Copy link
Contributor Author

Thanks for the changes. We'll probably add a default base gas that is consumed by calling evaporate instead of going the route of defaulting to 1 if 0 (1 gas is almost nothing, so just the host call probably consumes more relative CPU time), but at this point I think we can just merge this and tinker with it after? Any other comments @assafmo?

Ok, sounds good, thanks. Yeah, I realised that throwing an error is not really good behavior here but instead better to do some minimum.

@darwinzer0
Copy link
Contributor Author

Go tests = in https://github.com/scrtlabs/SecretNetwork/blob/master/x/compute/internal/keeper/secret_contracts_exec_test.go

I started Go tests here, but I don't know how to read the gas from ctx to actually test the amount that was used. ae4c375

@Cashmaney Cashmaney changed the base branch from master to evaporate March 29, 2023 15:09
@Cashmaney Cashmaney merged commit c9ca5a4 into scrtlabs:evaporate Mar 29, 2023
@Cashmaney
Copy link
Member

I went ahead and merged this into the evaporate branch. We'll finish the tests and add an API to query gas amounts here. Feel free to continue to review or build on ideas in this branch, and we'll tag you guys before the merge into master!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants