Define Consumer Contracts #33

ethanfrey · 2023-05-24T18:46:37Z

This will start some contract code from the consumer side. The intention is to clarify some interfaces in the documentation by means of sample code. But also, to work towards a minimal working virtual staking contract that can be used for system-level integrations with the virtual staking SDK module.

Define new interfaces between these contracts
Create minimal gov-set price-feed contract
Start native staking contract (no reward withdrawal)
Provide stub for the converter contract

hashedone

Some comments for current state.

contracts/consumer/converter/src/lib.rs

contracts/consumer/virtual-staking/src/contract.rs

hashedone · 2023-05-25T13:00:45Z

contracts/consumer/virtual-staking/src/contract.rs

+
+    fn handle_epoch(
+        &self,
+        deps: DepsMut<VirtualStakeCustomQuery>,


Ouch, I am not sure if it is properly supported in Sylvia to use custom queries, it was postponed. I will double-check, and if not, we will push for delivering it asap.

@jawoznia

I looked and the following are all not supported:

CustomQueries (Deps)

Exposing sudo in multitests (and maybe making another Sylvia method like reply to enable that)

I tried returning custom messages in instantiate and it works (at least the compiler didn't complain once I updated the lib.rs bindings... good to check this works with the auto-generated entry points)

For Custom Queries, note it could vary between T and Empty between calls, but not T and U. I guess all execute variants need the same, as well as all query variants, but eg instantiate may be DepsMut while execute is DepsMut

Good thing is sudo is outside of Sylvia so I can get this to compile and run for system level tests (uploading to a chain), even if I can't for multitest yet

NB: I rebased on your vault branch to check new Sylvia and it errors on returning Response<CustomMsg> from instantiate. (something about anyhow::Error so I assume mt related. One more thing to check then

Created CosmWasm/sylvia#139 and CosmWasm/sylvia#140

CosmWasm/sylvia#16 is the oldest open issue... time to knock it out.

Yea, both custom queries and custom messages are not supported. We will prioritize those.

Sudo is easy thing to do, for now we can do it in reply-wise and deprecate it when we properly implement them.

For Custom Queries, note it could vary between T and Empty between calls, but not T and U. I guess all execute variants need the same, as well as all query variants, but eg instantiate may be DepsMut while execute is DepsMut

I am aware, however it might happen we will enforce using T through entire contract due to practical reasons (it should not be a problem - Querier<T> can do everythin Querier<Empty> can do). Simillarly as with error type (you use ContractError even if StdError would be sufficient).

Not 100% sure how to approach it. This is too big PR to leave it not merged, but queries/responses would be in Sylvia this/next week (Jan is working on that right now). Sudo messages I can do in one day (not the way replies go, I have another approach for that which will stay relevant). On the other way, I assume it won't work properly so it is merging invalid code probably.

My take: change the reply to "default" for now, and where you query it just create a dummy default response, leaving todo: "Properly query when CosmWasm/sylvia#16 is merged". Then create issue on GH to fix it (to not forget).

Ahh, ignore it - it is used only in sudo. So you just use sudo this way which is working for now, except not testable at this point. But we will deliver it soon and then change. Good (for now).

contracts/consumer/virtual-staking/src/contract.rs

hashedone · 2023-05-25T13:04:33Z

contracts/consumer/virtual-staking/src/contract.rs

+    config: Item<'a, Config>,
+    // Amount of tokens that have been requested to bond to a validator
+    // (Sum of bond minus unbond requests). This will only update actual bond on epoch changes.
+    bond_requests: Map<'a, &'a str, Uint128>,


This key should probably be an &Addr

The key is a validator address (specifically valoper), which due to some weird architectural decision of the SDK uses a different bech32 prefix than the normal accounts, although it actually works the same way 🤷‍♂️

We have to treat as String without validating.

hashedone · 2023-05-25T13:15:09Z

contracts/consumer/virtual-staking/src/contract.rs

+        let requests: Vec<(String, Uint128)> = self
+            .bond_requests
+            .range(deps.storage, None, None, cosmwasm_std::Order::Ascending)
+            .collect::<Result<_, _>>()?;
+        let total_request: Uint128 = requests.iter().map(|(_, v)| v).sum();
+        if total_request > bond.cap.amount {
+            // TODO: normalize the list of requests to match the cap
+            todo!();
+        }
+
+        // TODO: Look at existing list and calculate differences for bond/unbond calls
+
+        // Save the new values
+        self.bonded.save(deps.storage, &requests)?;


Collecting all the requests, then summing them, then ignoring those out of range, and store after transformation seems waaaay off to me. I assume that normalizing requests is done to prevent wasting gas on transforming all the requests, but probably more gas is wasted by collecting all the requests, and always fetching/storing all the bonded items.

The algorithm should probably go ass follows:

The requests should never be collected, instead they should be transformed fold-like keeping the transformed items, and total request (both as Iterator::scan items). The reason for using scan instead of fold is, that fold is eager, and scan is lazy - after scan we can use take_while, to stop transforming after the cap is reached.

I would go even deeper, and in the scan I would just return an iterator over (request, total), then take_wile to break when total reaches cap, then map to transform items and update them immediately in bonded (which should be Map for that.

Happy for better design, but let me finish this, then improve. I need to:

Get all bond requests

Sum the total amount

If the sum <= max_cap then use collected requests as is

If the sum > max_cap,
a. calculate multiplier Decimal(max_cap / sum)
b. multiply every element of the collected requests in place.

Find diff between collected (normalized) requests and last bonding amounts (which go up, which down).

Transform diff into unbond and bond requests, sorting so all unbond happen first

The largest optimization I can see is collecting (in 1) into a {Hash,BTree}Map as we will likely need that later.

We don't cut off at the cap, we multiply everyone down. We need to get it all.

The storage load is the most expensive. Highest V is 100-150, so even O(N^2) in memory, which I would like to avoid, may be cheaper than the storage reads

contracts/consumer/virtual-staking/src/contract.rs

contracts/consumer/virtual-staking/src/types.rs

ethanfrey · 2023-05-25T17:20:50Z

The basic code is laid out. There are no tests.

Simple Price Feed is simple enough to be tested, and a good place to learn how, but not really needed.
The Virtual Staking contract, which really could use serious tests, is not testable with current Sylvia framework, as most logic happens in sudo entry point with CustomMsg and CustomQuery... all of which need new Sylvia features to test

However, this is a first step to getting something that @alpe could use in system tests (as the generated wasm should work)

ethanfrey · 2023-05-30T08:26:56Z

@hashedone I rebased on vault merge, can you review this (ideally run cargo build and cargo clippy locally).

The only issue I have is a build warning. Code:

use mesh_apis::local_staking_api::{self, LocalStakingApi, MaxSlashResponse};

#[contract]
#[messages(local_staking_api as LocalStakingApi)]
impl LocalStakingApi for NativeStakingContract<'_> {
 // ...
}

Error:

warning: unused import: `self`
 --> contracts/native-staking/src/local_staking_api.rs:6:36
  |
6 | use mesh_apis::local_staking_api::{self, LocalStakingApi, MaxSlashResponse};
  |                                    ^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

I guess the #messages[...] block is unused there? And I should remove it?

hashedone

Couple comments.

First of all - entry points can be generated with a single line from Sylvia - who commented on it twice, but there are more contracts and that apply to all of them.

The warning about unused modules is surprising, but it is not because #[messages(...)] is not used - it is always used, but when mt utils are not generated (so no mt flag enabled on Sylvia), the macro does not use the provided path. Don't remove the #[messages(...)] attribute; it will cause more problems in the future. For now, the best I can think of is adding #[allow(unused_imports)] just before the import block which causes this one. We will fix this warning in Sylvia by artificially using the module even when not generating things using it.

I am leaving the approval as I don't see any serious problem so you can merge when you fix or ignore comments.

contracts/consumer/converter/src/lib.rs

contracts/consumer/converter/src/contract.rs

contracts/consumer/simple-price-feed/src/contract.rs

contracts/consumer/simple-price-feed/src/lib.rs

hashedone · 2023-05-30T09:09:22Z

contracts/consumer/virtual-staking/src/contract.rs

+
+    fn handle_epoch(
+        &self,
+        deps: DepsMut<VirtualStakeCustomQuery>,


Not 100% sure how to approach it. This is too big PR to leave it not merged, but queries/responses would be in Sylvia this/next week (Jan is working on that right now). Sudo messages I can do in one day (not the way replies go, I have another approach for that which will stay relevant). On the other way, I assume it won't work properly so it is merging invalid code probably.

My take: change the reply to "default" for now, and where you query it just create a dummy default response, leaving todo: "Properly query when CosmWasm/sylvia#16 is merged". Then create issue on GH to fix it (to not forget).

contracts/consumer/virtual-staking/src/contract.rs

hashedone · 2023-05-30T09:40:17Z

contracts/consumer/virtual-staking/src/contract.rs

+
+    fn handle_epoch(
+        &self,
+        deps: DepsMut<VirtualStakeCustomQuery>,


Ahh, ignore it - it is used only in sudo. So you just use sudo this way which is working for now, except not testable at this point. But we will deliver it soon and then change. Good (for now).

ethanfrey · 2023-06-01T07:17:18Z

Made requested fixes and rebased on main.
Will merge when this passes CI

ethanfrey force-pushed the start-consumer-apis branch from c0ec886 to 14c29ae Compare May 24, 2023 19:17

ethanfrey requested a review from hashedone May 24, 2023 19:33

hashedone reviewed May 25, 2023

View reviewed changes

ethanfrey force-pushed the start-consumer-apis branch from 4b1f611 to 33e5cb7 Compare May 25, 2023 16:19

ethanfrey marked this pull request as ready for review May 25, 2023 17:18

ethanfrey force-pushed the start-consumer-apis branch from 8ff516c to 25b07cb Compare May 30, 2023 08:24

hashedone approved these changes May 30, 2023

View reviewed changes

ethanfrey added 10 commits June 1, 2023 09:15

Added placeholders packages for new consumer contracts

aadf2ea

Define converter APIs

403262a

Define basic implementation of virtual staking contract

884055a

Start working with custom msg/query

8e243a2

Address PR comments

c6f5fbe

Implement epoch handing minus reward payout

2fdcbd6

Write stub for converter contract

9f7b112

Implement basic price feed contract

021379a

Fix compiler errors

f64111a

Fixup from PR comments

53ac9e6

ethanfrey force-pushed the start-consumer-apis branch from 25b07cb to 53ac9e6 Compare June 1, 2023 07:15

ethanfrey merged commit bf2c73c into main Jun 1, 2023

ethanfrey deleted the start-consumer-apis branch June 1, 2023 07:19

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Define Consumer Contracts #33

Define Consumer Contracts #33

ethanfrey commented May 24, 2023 •

edited

Loading

hashedone left a comment

hashedone May 25, 2023

ethanfrey May 25, 2023

ethanfrey May 25, 2023

ethanfrey May 25, 2023

ethanfrey May 25, 2023

hashedone May 26, 2023 •

edited

Loading

hashedone May 30, 2023

hashedone May 30, 2023

hashedone May 25, 2023

ethanfrey May 25, 2023

hashedone May 25, 2023

ethanfrey May 25, 2023

ethanfrey commented May 25, 2023

ethanfrey commented May 30, 2023

hashedone left a comment

hashedone May 30, 2023

hashedone May 30, 2023

ethanfrey commented Jun 1, 2023

Define Consumer Contracts #33

Define Consumer Contracts #33

Conversation

ethanfrey commented May 24, 2023 • edited Loading

hashedone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hashedone May 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethanfrey commented May 25, 2023

ethanfrey commented May 30, 2023

hashedone left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ethanfrey commented Jun 1, 2023

ethanfrey commented May 24, 2023 •

edited

Loading

hashedone May 26, 2023 •

edited

Loading