Adding CircSupply Calculations #710

RajarupanSampanthan · 2020-09-21T18:45:16Z

Summary of changes
Changes introduced in this pull request:

Ports over CircSupply related functionality from Lotus

Reference issue to close (if applicable)

Closes #677

Other information and links

austinabell

This logic should be replacing that in total_fil_circ_supply, this isn't connected in any way.

Also there is a lot of duplicate logic, try to create abstractions to reduce this, somewhat like how it is done in total_fil_circ_supply currently

blockchain/state_manager/src/lib.rs

ec2 · 2020-09-23T19:04:48Z

Building on what Austin said, I dont see why you need the ChainStore in the first place? I dont see any indications that the ChainStore is needed. Every time cs is used, you're using cs.db, which indicates to me you really only want the database (which is already in the StateManager) and not the ChainStore.

blockchain/state_manager/src/utils.rs

austinabell

I don't think incomplete network upgrade logic is worth bringing in, especially since untested. Can you please just remove it and just have the calculation based on the original and add TODOs where necessary? It's unclear we will even support this upgrade

types/src/lib.rs

vm/actor/src/builtin/power/state.rs

vm/actor/src/builtin/reward/state.rs

vm/interpreter/src/circ_supply.rs

vm/interpreter/src/vm.rs

Co-authored-by: Austin Abell <austinabell8@gmail.com>

ec2 · 2020-10-15T19:18:57Z

Most of this stuff should probably live in the state manager. This is because there will API endpoint implemented for most/all of these functions.

austinabell · 2020-10-26T15:17:50Z

vm/interpreter/src/vm.rs

+        let default_preignition =
+            setup_preignition_genesis_actors_testnet(self.store).unwrap_or_default();
+        let default_postignition =
+            setup_postignition_genesis_actors_testnet(self.store).unwrap_or_default();


This logic should not be inside of the VM. Also, this is being called every time there is a send in the VM, terribly inefficient

I think this should be passed into the VM::new() as a function pointer right? I believe that was the whole point of that change.

Well, acutally I dont meant that directly. It should be called every time you do a get_vm_circulating_supply. and that is what should be passed as the funciton pointer

The circ supply calculation should yes, this is not really related to that, and this function should not be called every time that is called, only when it's not initialized

austinabell · 2020-10-26T15:18:24Z

vm/interpreter/src/default_runtime.rs

+        pre_ignition: GenesisInfo,
+        post_ignition: GenesisInfo,


This should not be attached to the runtime, this does not allow for swapable circ supply calculations

tests/conformance_tests/tests/conformance_runner.rs

…zed genesis for state manager

…pply

blockchain/state_manager/src/vm_circ_supply.rs

austinabell

did some cleanup after your review @ec2 if you want to double check

ec2 · 2020-10-28T15:35:01Z

looked at the changes, lgtm

RajarupanSampanthan added 3 commits September 21, 2020 12:00

First commit

19facfd

Proper error handling

530b144

Added proper error messages and fixed lint issues

120a8aa

RajarupanSampanthan requested review from ansermino, austinabell, dutterbutter and ec2 as code owners September 21, 2020 18:45

austinabell added Blockchain Status: Needs Review labels Sep 22, 2020

austinabell suggested changes Sep 22, 2020

View reviewed changes

blockchain/state_manager/src/lib.rs Outdated Show resolved Hide resolved

blockchain/state_manager/src/lib.rs Outdated Show resolved Hide resolved

austinabell added Status: Needs Changes and removed Status: Needs Review labels Sep 22, 2020

ec2 requested changes Sep 23, 2020

View reviewed changes

blockchain/state_manager/src/utils.rs Outdated Show resolved Hide resolved

RajarupanSampanthan added 9 commits September 24, 2020 20:54

Merge branch 'main' into rupan/curc_supply

a8e5858

Moving circ supply to interpreter

03c0e69

Remvoing old logic

0cc3b18

Updating

227c520

TRansfer test failing

ad5f58f

Merge branch 'main' into rupan/curc_supply

62d7c20

Update

2c88854

Merge branch 'main' into rupan/curc_supply

6dae6d4

Merge branch 'main' into rupan/curc_supply

acf4820

RajarupanSampanthan added Status: Needs Review and removed Status: Needs Changes labels Oct 8, 2020

austinabell suggested changes Oct 14, 2020

View reviewed changes

RajarupanSampanthan and others added 2 commits October 15, 2020 12:19

Update vm/actor/src/builtin/power/state.rs

c8d9ec6

Co-authored-by: Austin Abell <austinabell8@gmail.com>

Update vm/actor/src/builtin/reward/state.rs

28bc2d6

Co-authored-by: Austin Abell <austinabell8@gmail.com>

Merge branch 'main' into rupan/curc_supply

3467d2c

RajarupanSampanthan added 6 commits October 22, 2020 13:23

Merge branch 'main' into rupan/curc_supply

0dc43d6

Weird conformance issue

6f18eaf

Update

9733550

Fixed serialization prob

5ecdbad

Update

06031dc

Merge branch 'main' into rupan/curc_supply

63324a0

RajarupanSampanthan requested review from ec2 and austinabell October 26, 2020 15:00

austinabell suggested changes Oct 26, 2020

View reviewed changes

RajarupanSampanthan and others added 8 commits October 26, 2020 12:48

Still need to fix curc supply func

898a049

Having lifetime issues

c1d0525

Fix trait usage and cleanup (wip still need to fix genesis init)

b177d13

Handle exact functionality of Lotus and avoid requirement on initiali…

bf4afe9

…zed genesis for state manager

Merge branch 'main' of github.com:ChainSafe/forest into rupan/curc_su…

1950d93

…pply

Remove commented out code

760ba78

Remove other commented out code

d03c227

clippy

c0568f6

austinabell added Status: Needs Review and removed Status: Needs Changes labels Oct 27, 2020

Remove unused code

1114355

ec2 approved these changes Oct 28, 2020

View reviewed changes

blockchain/state_manager/src/vm_circ_supply.rs Outdated Show resolved Hide resolved

blockchain/state_manager/src/vm_circ_supply.rs Outdated Show resolved Hide resolved

blockchain/state_manager/src/vm_circ_supply.rs Show resolved Hide resolved

ec2 requested review from timvermeulen and StaticallyTypedAnxiety October 28, 2020 02:19

RajarupanSampanthan and others added 3 commits October 28, 2020 00:41

Adding ec2 requested changes

aae69bd

Remove unused deps

a25160c

Cleanup

be22795

austinabell approved these changes Oct 28, 2020

View reviewed changes

austinabell merged commit 2e1bb09 into main Oct 28, 2020

austinabell deleted the rupan/curc_supply branch October 28, 2020 16:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding CircSupply Calculations #710

Adding CircSupply Calculations #710

RajarupanSampanthan commented Sep 21, 2020

austinabell left a comment

ec2 commented Sep 23, 2020

austinabell left a comment

ec2 commented Oct 15, 2020

austinabell Oct 26, 2020

ec2 Oct 26, 2020

ec2 Oct 26, 2020

austinabell Oct 26, 2020

austinabell Oct 26, 2020

austinabell left a comment

ec2 commented Oct 28, 2020

Adding CircSupply Calculations #710

Adding CircSupply Calculations #710

Conversation

RajarupanSampanthan commented Sep 21, 2020

austinabell left a comment

Choose a reason for hiding this comment

ec2 commented Sep 23, 2020

austinabell left a comment

Choose a reason for hiding this comment

ec2 commented Oct 15, 2020

austinabell Oct 26, 2020

Choose a reason for hiding this comment

ec2 Oct 26, 2020

Choose a reason for hiding this comment

ec2 Oct 26, 2020

Choose a reason for hiding this comment

austinabell Oct 26, 2020

Choose a reason for hiding this comment

austinabell Oct 26, 2020

Choose a reason for hiding this comment

austinabell left a comment

Choose a reason for hiding this comment

ec2 commented Oct 28, 2020