Merge investor into TOCC #525

branan · 2021-11-23T23:34:57Z

No description provided.

…r-hacking

This expands `close_epoch` to have most of the top-level epoch closing logic, with most of the meat still stubbed out.

These extrinsics are really an interface to do fake borrowing/payback until we have a borrower side to interact with. They are renamed to indicate now that that is what they do.

This removes some confusing code where the terms `pool` or `tranche` were being used to refer to the id in some cases but the struct data in others. IDs are now always delimited as such

* Add Reserve * wip: checkpoint epoch validation * checkpoint - sub ratio validation * checkpoint - return errors + simplify balance to perquintil conversion * address comments 1 * address comments 2 * moved currency/tranchetoken as primitive crate

* Integrates crowdloan pallets to altait * Pump rt-version 1005, minDep democ 1000 AIR

…e#474) * Changed validate-unsigned The changes to the validate unsigned are here to reflect more detailed error messages at failure with the costum error instead of the misleading 'BadProof' return value. Furthermore, this fixes an issue, where somebody could have a signature of any parachain-relay-chain account and then use a proof of any other relay-chain contribution to gain those rewards. * Enable claim_reward calls * Remove indicator for claim failures

* Epoch execution phase 1 This is the beginning of epoch execution. It updates tranche state and stores the execution results, but since token pricing is not yet implemented it cannot do rebalancing or other bits like that. * Epoch close pricing calculation This now calculates a price (including dripping interest) for the tranches at epoch close. There are other locations where dripping interest needs to occur, to come in further commits. * Store additional state at epoch close In addition to the per-tranche supply/redeem targets for an epoch, we now also store the nav, reserve, per-tranche price, and per-tranche total value at the time of close. * Update epoch validation tests for new epoch storage model * Implement tranche rebalancing during epoch execution This also includes a new `fake_nav` to make testing easier * Track tranche debt/balance amounts in borrow/repay fns * Account for updated tranche value in epoch validity * Fixes for epoch execution / rebalancing * Epoch execution testing * Use a less-bad pow() impl for calculating interest * Avoid rounding issues when round-tripping between currencies

branan · 2021-11-23T23:36:41Z

Things left to do:

Create a mock reserve so that loan tests don't rely on pool pallet
Unify pool creation within the pool pallet

NunoAlexandre · 2021-11-29T18:42:43Z

@branan there are some merge conflicts; and is this PR ready for review?

vedhavyas

Some questions on the tranche balancing

vedhavyas · 2021-11-26T08:23:18Z

pallets/tinlake-investor-pool/src/lib.rs

+
+#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, Default)]
+pub struct Tranche<Balance> {
+	pub interest_per_sec: Perquintill,


any specific reason why we are not using substrate fixed point types?

Looks like this needs to be using 27 precision instead of 18

Perquintill is a fixed point type, technically. It's the semantically-correct type for a number which cannot exceed 1.0.

This only stores the base interest rate and is not used as an accumulator. Intermediate values for interest calculations do use 27 decimals. I think 18 decimals is sufficient here, but if we really want to store 27 we should make a Peroctill type

I think we should use the 27 precision

That's fine. I'll add an item to the things-to-do in December to add a Peroctill we can use for 27-decimal proper fractions and update this. Should be a one-liner in our substrate fork and here.

Doesn't this mean we will have to use the fork of substrate again?

We're already still using a fork for the other 27-digit fixed points, right? Or did we finally get that upstream?

I may actually use Fixed27 instead of Peroctill, so that the interest is in the form of 1.<rate>. it saves one addition for every interest calculation

We ware using the main repo on parachain at least. Would prefer to keep it that way 😄

We dont use the fork anymore. I have moved the macro to our repo. You can already use the macro from runtime-common

vedhavyas · 2021-11-30T14:37:12Z

pallets/tinlake-investor-pool/Cargo.toml

@@ -0,0 +1,49 @@
+[package]
+authors = ['Substrate DevHub <https://github.com/substrate-developer-hub>']


I think this Cargo toml needs an update

Yup, still copied from the template

I'll do a separate PR doing an update here as well?

pallets/tinlake-investor-pool/src/lib.rs

vedhavyas · 2021-11-30T14:43:04Z

pallets/tinlake-investor-pool/src/lib.rs

+		) -> DispatchResult {
+			let owner = ensure_signed(origin)?;
+
+			// TODO: Ensure owner is authorized to create a pool


not sure if this is still valid

Yeah, this gets lifted out into the pool pallet now, I believe

vedhavyas · 2021-11-30T14:46:50Z

pallets/tinlake-investor-pool/src/lib.rs

+								.map_err(|_| Error::<T>::TrancheId)?,
+						};
+						Self::update_tranche_for_epoch(
+							loc,


I think we have to handle the execution failure so that we have an way to close it through a call

update_tranche_for_epoch is infallible here, since it can only fail on transfers and has no transfers to make. I can leave a comment to that effect (or add a .expect())

vedhavyas · 2021-11-30T14:53:06Z

pallets/tinlake-investor-pool/src/lib.rs

+					tranche_id: T::TrancheId::try_from(tranche_id)
+						.map_err(|_| Error::<T>::TrancheId)?,
+				};
+				Self::update_tranche_for_epoch(


I think we should handle the failed case down the line for theese storage changes

Here it probably makes sense to catch that the transfer failed and fail cleanly in one place, with the pool marked as "broken" somehow to prevent further epoch operations until we can fix it with sudo somehow.

That's a big enough change it probably deserves it's own PR separately from the merge work?

vedhavyas · 2021-11-30T15:31:41Z

pallets/tinlake-investor-pool/src/lib.rs

+					tranche.debt = remaining_nav;
+					tranche.reserve = remaining_reserve;
+				} else {
+					tranche.debt = ratio.mul_ceil(nav);


shouldn;t this be the debtratio instead of tranch share of the pool ?

We don't track separate debt and reserve ratios. We track a single pool ratio, and the goal of this step is to readjust the debt/reserve values of each tranche based on the new debt/reserve totals and the tranche's recomputed ratio from the epoch close.

branan · 2021-11-30T16:43:49Z

@branan there are some merge conflicts; and is this PR ready for review?

@NunoAlexandre Just needs a rebase on the last couple commits from the borrower benchmarking, it's otherwise ready to go

NunoAlexandre · 2021-11-30T16:56:29Z

@branan there are some merge conflicts; and is this PR ready for review?

@NunoAlexandre Just needs a rebase on the last couple commits from the borrower benchmarking, it's otherwise ready to go

Do we have a spec this PR is implementing that I can look to for a basis of review?

branan · 2021-11-30T17:21:38Z

@NunoAlexandre The point of this PR was really just the merge, which doesn't have its own standalone spec.

It is, however, a convenient place for folks that haven't had eyes on the lender side to comment on things. The overall lender spec is at https://centrifuge.hackmd.io/l1HoQDTUSHKDO8-HbD0NaQ, and the original Tinlake components are the lender folder in the tinlake contracts - especially the coordinator and assessor contracts.

branan · 2021-11-30T17:25:47Z

I'm trying not to clutter this PR with changes to the underlying Lender work right now, since the goal here is to get everything in one branch to make continued development more straightforward. I'd like to see this get through once the merge part is unobjectionable, and issues can be filed to fix up anything that folks find while going over this.

vedhavyas · 2021-11-30T18:10:19Z

@branan it is fine to merge as is but I would suggest we add these relevent issue as either todo(s), or actual issues that we can actively track. Also helps us to what to pick immediately next

branan · 2021-11-30T18:22:56Z

Yes, will absolutely file real tickets to track it all.

I'll resolve the merge conflicts momentarily and get things ticketed out this afternoon.

branan · 2021-12-01T02:48:56Z

This is re-re-based on top of the TOCC branch with the updates from the final borrower benchmarking PR

vedhavyas · 2021-12-01T08:09:50Z

@branan okay, merge this up when you are ready. We can talk next steps in sync then :-D

mustermeiszer

This is a bit late, but regarding the some important things I wanted to remark here.

mustermeiszer · 2021-12-06T09:23:56Z

.github/workflows/build.yml

+      - if: matrix.target == 'build-runtime'
+        name: Setup - gcloud / gsutil
+        uses: google-github-actions/setup-gcloud@master
+        with:
+          service_account_key: ${{ secrets.GCS_SA_KEY }}
+          project_id: ${{ secrets.GCS_PROJECT }}
+          export_default_credentials: true
+      - if: matrix.target == 'build-runtime'
+        name: Publish to GCS
+        run: |
+          gsutil cp ./runtime/altair/target/srtool/release/wbuild/altair-runtime/altair_runtime.compact.wasm gs://centrifuge-artifact-releases/parachain/altair_runtime-$(git rev-parse --short HEAD).compact.wasm


Where and why do we store the wasms now?

I think @mikiquantum wrote this originally? I'm not sure why we decided to push things to google storage.

mustermeiszer · 2021-12-06T09:28:04Z

pallets/tinlake-investor-pool/src/lib.rs

+
+#[derive(Clone, Encode, Decode, Eq, PartialEq, RuntimeDebug, Default)]
+pub struct Tranche<Balance> {
+	pub interest_per_sec: Perquintill,


Doesn't this mean we will have to use the fork of substrate again?

pallets/tinlake-investor-pool/src/lib.rs

mustermeiszer · 2021-12-06T10:16:13Z

pallets/tinlake-investor-pool/src/lib.rs

+			Order::<T>::try_mutate(&tranche, &who, |order| -> DispatchResult {
+				if amount > order.redeem {
+					let transfer_amount = amount - order.redeem;
+					Pool::<T>::try_mutate(pool_id, |pool| {
+						let pool = pool.as_mut().ok_or(Error::<T>::NoSuchPool)?;
+						let epoch_redeem = &mut pool.tranches[tranche_id.into()].epoch_redeem;
+						*epoch_redeem = epoch_redeem
+							.checked_add(&transfer_amount)
+							.ok_or(Error::<T>::Overflow)?;
+						T::Tokens::transfer(currency, &who, &pool_account, transfer_amount)
+					})?;
+				} else if amount < order.redeem {
+					let transfer_amount = order.redeem - amount;
+					Pool::<T>::try_mutate(pool_id, |pool| {
+						let pool = pool.as_mut().ok_or(Error::<T>::NoSuchPool)?;
+						let epoch_redeem = &mut pool.tranches[tranche_id.into()].epoch_redeem;
+						*epoch_redeem = epoch_redeem
+							.checked_sub(&transfer_amount)
+							.ok_or(Error::<T>::Overflow)?;
+						T::Tokens::transfer(currency, &pool_account, &who, transfer_amount)
+					})?;
+				}
+				order.redeem = amount;
+				order.epoch = epoch;
+				Ok(())
+			})
+		}


Frontrunning as mentioned above.

mustermeiszer · 2021-12-06T10:22:11Z

pallets/tinlake-investor-pool/src/lib.rs

+				// If closing the epoch would wipe out a tranche, the close is invalid.
+				// TODO: This should instead put the pool into an error state
+				ensure!(
+					!epoch_tranche_prices
+						.iter()
+						.any(|price| *price == Zero::zero()),
+					Error::<T>::WipedOut
+				);


When does this potentially happen?

This should only happen if the NAV has dropped precipitously (due to defaults or something like that)

mustermeiszer · 2021-12-06T10:28:16Z

pallets/tinlake-investor-pool/src/lib.rs

+			let pool_address = PoolLocator {
+				pool_id: loc.pool_id,
+			}
+			.into_account();


See comment on PoolLocatpr above.

mustermeiszer · 2021-12-06T10:29:22Z

pallets/tinlake-investor-pool/src/lib.rs

+			pool_id: T::PoolId,
+			amount: T::Balance,
+		) -> DispatchResult {
+			let pool_account = PoolLocator { pool_id }.into_account();


See comment on PoolLocator above.

pallets/tinlake-investor-pool/src/lib.rs

Branan Riley and others added 27 commits June 28, 2021 19:43

Add skeleton of Tinlake investor pallet

27ab63c

Remove example content from investor-pool pallet

46fd1ba

Switch to Perquintill for interest calculation

312f842

Add tokens and initial supply/redeem order support

a6e72d4

Give the pool a direct parameter for the CurrencyId

696edaa

Add beginning of epoch closing logic

a0b74f2

Fix epoch details - it contains percentages/ratios, not absolute values.

fcfc674

Add Reserve (centrifuge#444)

dcb4e94

Epoch close tweaks

fc91d54

Merge remote-tracking branch 'upstream/investor-hacking' into investo…

087a9a2

…r-hacking

Build the epoch execution skeleton

b645c71

This expands `close_epoch` to have most of the top-level epoch closing logic, with most of the meat still stubbed out.

Improve errors + events for investor pallet

5f629ac

Unify pool + reserve logic; remove per-tranche accounts

0999255

Rename deposit/payout extrinsics

18ddce0

These extrinsics are really an interface to do fake borrowing/payback until we have a borrower side to interact with. They are renamed to indicate now that that is what they do.

Add doc comments to some less-obvious pieces

fae369a

Be more consistent in use of pool_id/tranche_id

9ce1ab9

This removes some confusing code where the terms `pool` or `tranche` were being used to refer to the id in some cases but the struct data in others. IDs are now always delimited as such

Remove unused traits

ef1537e

Use UnixTime trait for time instead of Timestamp pallet directly

9f77060

Add Epoch Validation (centrifuge#471)

2cb0169

* Add Reserve * wip: checkpoint epoch validation * checkpoint - sub ratio validation * checkpoint - return errors + simplify balance to perquintil conversion * address comments 1 * address comments 2 * moved currency/tranchetoken as primitive crate

RTU1 + srtool push (centrifuge#487)

fb6c8eb

fix build

424de85

add workflow dispatch for docker (centrifuge#492)

566c022

Integrates Crowdloan Pallets into Altair (centrifuge#493)

d9d8454

* Integrates crowdloan pallets to altait * Pump rt-version 1005, minDep democ 1000 AIR

Merge branch 'parachain' into investor-hacking

5913f6b

Merge remote-tracking branch 'upstream/parachain' into investor-hacking

c16c78f

This comment has been minimized.

Sign in to view

branan force-pushed the merge-investor branch from 0c951cf to 8f3dc69 Compare November 28, 2021 22:11

vedhavyas reviewed Nov 30, 2021

View reviewed changes

Branan Riley added 3 commits November 30, 2021 16:07

Merge branch 'investor-hacking' into tocc

ef47790

Use investor pallet as reserve source for loan pallet

828dc60

Use Loan pallet as NAV source for investor pool

5b09f5e

branan force-pushed the merge-investor branch from 8f3dc69 to 5b09f5e Compare December 1, 2021 00:26

branan merged commit 38926d3 into centrifuge:tocc Dec 1, 2021

mustermeiszer reviewed Dec 6, 2021

View reviewed changes

		@@ -0,0 +1,49 @@
		[package]
		authors = ['Substrate DevHub <https://github.com/substrate-developer-hub>']

Merge investor into TOCC #525

Merge investor into TOCC #525

Conversation

branan commented Nov 23, 2021

branan commented Nov 23, 2021

This comment has been minimized.

NunoAlexandre commented Nov 29, 2021

vedhavyas 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vedhavyas Dec 9, 2021 • 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

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

branan commented Nov 30, 2021

NunoAlexandre commented Nov 30, 2021

branan commented Nov 30, 2021

branan commented Nov 30, 2021

vedhavyas commented Nov 30, 2021

branan commented Nov 30, 2021

branan commented Dec 1, 2021

vedhavyas commented Dec 1, 2021

mustermeiszer 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vedhavyas Dec 9, 2021 •

edited

Loading