Skip to content
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

Update Miner actor #691

Merged
merged 44 commits into from
Sep 26, 2020
Merged

Update Miner actor #691

merged 44 commits into from
Sep 26, 2020

Conversation

timvermeulen
Copy link
Contributor

Closes #559

commit 8ef5ae5
Author: Austin Abell <austinabell8@gmail.com>
Date:   Wed Sep 16 12:30:01 2020 -0400

    Peer stats tracking and selection (#701)

    * Added peer manager logic and started to move blocksync handling into network context

    * Migrate to using handled blocksync requests and remove a duplicate code

    * Fix test use

    * Fix duration subtract logic

    * Update peer manager updates from hello requests

    * Docs and cleanup

    * more cleanup

commit dc0ff4c
Author: Eric Tu <6364934+ec2@users.noreply.github.com>
Date:   Tue Sep 15 14:43:18 2020 -0400

    Semantic Validation for Messages (#703)

    * semantic validation

    * suggestions

    * suggestions

commit 3411459
Author: Austin Abell <austinabell8@gmail.com>
Date:   Mon Sep 14 15:14:48 2020 -0400

    ChainSync refactor (#693)

    * Switch chain store to threadsafe

    * Update genesis to arc reference

    * wip refactoring chain sync to workers in async tasks

    * Update network event handling and remove NetworkHandler

    * Update tipset scheduling logic

    * Update peer retrieval to take a random sample of available peers

    * Cleanup and enabling all existing tests

    * fix worker task spawn

    * Add TODO for emit event ignoring and change to error log

    * oops

    * Update comment

    * Fix typo

commit 66ca99e
Author: Eric Tu <6364934+ec2@users.noreply.github.com>
Date:   Mon Sep 14 13:48:22 2020 -0400

    Fix StateManager use in different components (#694)

    * wrap sm in arc

    * lint

    * fix tests

    * suggestions

    * clippy

commit 96b64cb
Author: nannick <rajarupans@gmail.com>
Date:   Mon Sep 14 10:33:27 2020 -0400

    Drand ignore env variable (#697)

    * First commit

    * Using enviromental vairable instead of cli flag

    * FIxing dumb mistakes

    * Renaming drand flag

    * UPdate latest_beacon_entry

    * Adding drand check

    * Adding doc

commit 548a464
Author: Austin Abell <austinabell8@gmail.com>
Date:   Thu Sep 10 13:22:22 2020 -0400

    Print out conformance results and add log for skips (#695)

commit 0d7b16c
Author: Stepan <s.s.naumov@gmail.com>
Date:   Tue Sep 8 09:01:05 2020 -0400

    Add CLI command to add Genesis Miner to Genesis Template (#644)

    * AddMinerGenesis cmd command

    * fixing tests

    * addressing comments

    * removing jsons
# Conflicts:
#	blockchain/chain_sync/src/sync_worker.rs
@timvermeulen timvermeulen marked this pull request as ready for review September 21, 2020 14:56
# Conflicts:
#	blockchain/chain_sync/src/sync_worker.rs
#	blockchain/state_manager/src/utils.rs
#	node/rpc/src/state_api.rs
vm/actor/src/builtin/miner/miner_state.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/miner_state.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/miner_state.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/miner_state.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/miner_state.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/miner_state.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/miner_state.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/miner_state.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/miner_state.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/miner_state.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@austinabell austinabell left a comment

Choose a reason for hiding this comment

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

Going to break this one up because I need to take a break and there are some common issues

vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
let empty_deadline_cid = rt
.store()
.put(&Deadline::new(empty_array.clone()), Blake2b256)
.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

These should not be unwrapped, they could hit an out of gas error, which should not crash

let empty_deadlines_cid = rt
.store()
.put(&Deadlines::new(empty_deadline_cid), Blake2b256)
.unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

same

.put(&Deadlines::new(empty_deadline_cid), Blake2b256)
.unwrap();

let empty_vesting_funds_cid = rt.store().put(&VestingFunds::new(), Blake2b256).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

same

vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/types.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/mod.rs Outdated Show resolved Hide resolved
types/src/sector/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/monies.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/partition_state.rs Show resolved Hide resolved
vm/actor/src/builtin/miner/policy.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/policy.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/sector_map.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/miner/sectors.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/power/mod.rs Outdated Show resolved Hide resolved
vm/actor/src/builtin/power/types.rs Show resolved Hide resolved
Comment on lines 149 to 161
let empty_bitfield_cid = rt.store().put(&BitField::new(), Blake2b256).unwrap();

let empty_deadline_cid = rt
.store()
.put(&Deadline::new(empty_array.clone()), Blake2b256)
.unwrap();

let empty_deadlines_cid = rt
.store()
.put(&Deadlines::new(empty_deadline_cid), Blake2b256)
.unwrap();

let empty_vesting_funds_cid = rt.store().put(&VestingFunds::new(), Blake2b256).unwrap();
Copy link
Contributor

Choose a reason for hiding this comment

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

These errors are still not downcasted, and no reason not too, since a direct interaction with the store. It's okay for the error to be incorrect until I make another pass, but I'd really like to avoid a potential crash when using.

Can probably downcast all of these errors at once if you put this logic in a smaller scope

@timvermeulen timvermeulen merged commit f007210 into main Sep 26, 2020
@timvermeulen timvermeulen deleted the tim/miner_actor branch September 26, 2020 15:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Miner actor
3 participants