onflow / flow-core-contracts Goto Github PK

Issue To Be Solved

Need developer docs for the staking collection contract

Suggest A Solution

• Do a short write-up that helps developers in the community get comfortable with it so they can help review the PR.

• Write documentation that replaces the staking with unlocked FLOW guide: https://docs.onflow.org/staking/unlocked-staking-guide/

• The documentation should describe how to use the staking collection contract in the same style as the existing docs

• This also applies to locked FLOW, so find a way to merge the two docs into one so all can see it

Issue To Be Solved

Need to be able to calculate new rewards payout every epoch and save as as part of epoch metadata

Suggest A Solution

• Work off the feature/epochs branch
• Add a field to epoch metadata for the reward amount
• Add a field to configurable metadata for reward APY
• Admin can update APY field
• When paying rewards, calculate the new APY

Issue To Be Solved

Rewards might need to be paid during a different phase, so it shouldn't be part of the start new epoch function.

Suggest A Solution

• Create new payRewards function for epochs that is separate
• Allow this function to be called by the epoch admin directly
• Test transaction

Issue To Be Solved

If you want to know the status of a NodeRecord right now you have to call lots of methods to fetch different fields. Having the ability to return a single NodeRecordStatus struct of a map of all of them would make it easier to write scripts on top of the api to show what is staked.

(Optional): Suggest A Solution

Create a NodeRecordStatus struct with calculated and safe properties and return that. Kind like versus does it here: https://github.com/versus-flow/auction-flow-contract/blob/master/contracts/Auction.cdc#L13

(Optional): Context

Nothing, just want a nice API.

Context

For epochs, the admin needs to ability to enable and disable staking so that EpochSetup and EpochCommitted actions can be performed without staking transactions operating at the same time.

Definition of Done

• Add stakingEnabled: Bool field to staking contract
• Add admin function to enable and disable it
• Add utility function to check if it is enabled
• Add pre-check to every staking transaction to make sure that staking is enabled before preceding (except for withdrawing tokens)
• Add pre-check for pay rewards and move tokens to make sure staking is disabled before being called.

Required Tests

• Make sure each staking operation fails when disabled
• Make sure admin operations fail when enabled

Issue To Be Solved

Need to be able to calculate rewards for an epoch and pay at separate times

Suggest A Solution

• Change payRewards to calculateRewards and have it return a dictionary mapping nodes and delegators to their reward amounts
• Add a payRewards function to the the epoch contract to pay rewards to all the nodes from the previous epoch
• Add a function to scale the rewards of a staker if needed for slashing an such
• Add a function to scale all the rewards of all stakers in case the epoch was short or long
• Only store staking information for the previous epoch

Issue To Be Solved

The epoch smart contracts currently enforce that QC and DKG resources can only be created during epoch setup. They should be able to be created at any time.

Suggest A Solution

• Work from the flow/epochs branch
• Remove restriction in the QC contract that QC has to be enabled to create a Voter
• Remove restriction in the DKG contract the DKG has to be enabled to create a Participant
• Remove the phase restriction in FlowEpoch contract for creating QC Voter and DKG Participant
• Test that these can be created at any time

Document rewards in the flow-docs website

From the FLIP (onflow/flow#99):

Each account will have a public StorageCapacity.depositStorage capability and a private StorageCapacity resource on predetermined paths.

The StorageCapacity resource defined on the StorageFees smart contract:

• has a storageCapacity field which defines how much storage can the account holding this resource use.
• has a FlowVault with Flow that was used for paying for this storage. This vault cannot be accessed by anyone except the StorageFees smart contract. The exception is the balance of this flow vault which should be readable by anyone.
• has an array of pairs of UInt64, UFix64 purchases used for keeping track of how much storage was purchased for how much flow.
• has a depositStorage function that can only be used by the StorageFees smart contract (is this possible?):
  • it should increment the storageCapacity field deposit into the FlowVault and append to purchases

The StorageFees smart contract will also have the following responsibilities:

• Definitions:
  • minimumStorageUnit: define the minimum unit (chunk) size of storage in bytes. Storage can only be bought (or refunded) in a multiple of the minimum storage unit.
  • minimumAccountStorage: defines the minimum amount of storage_capacity an address can have and also the amount every new account has. minimumAccountStorage is a multiple of minimumStorageUnit.
  • flowPerByte: defines the cost of 1 byte of storage in FLOW tokens.
  • flowPerAccountCreation: define the cost of purchasing the initial minimum storage in FLOW tokens.
  • refundingEnabled: defines if accounts can refund storage
• minimumAccountStorage, refundingEnabled, flowPerByte and flowPerAccountCreation should be able to change in the future.
• Functions:
  • setupStorageForAccount(paymentVault, authAccount):
    • The paymentVault should have funds for the minimum storage needed by the address.
    • Check if StorageCapacity is already present on the account (if so panic; account already bought some storage).
    • Create the StorageCapacity resource with minimumAccountStorage and put the paymentVault into it.
    • Put this StorageCapacity resource onto the account with a corresponding receiver.
  • purchaseStorageForAddress(paymentVault, address, amount):
    • The payer pays for extra storage needed by the address.
    • Check if StorageCapacity is present on the address (if not so panic; account needs to buy initial storage first).
    • Check if amount is divisible by minimumStorageUnit (if not panic).
    • Check if correct amount of funds were provided by paymentVault. Should be flowPerByte x amount
    • Using StorageCapacity.depositStorage on the destination account, increment its StorageCapacity.storageCapacity and deposit paymentVault into StorageCapacity.FlowVault.
  • refundStorage(authAccount, amount):
    • The account refunds its own purchased storage to its own vault
    • Check if StorageCapacity is present on the address (if not so panic; account needs to buy initial storage first)
    • Check if amount is divisible by minimumStorageUnit (if not panic)
    • Check if this would put account below minimumAccountStorage (if so panic)
    • If account refunds too much storage capacity (it is using more than it has left) the transaction will fail anyway, so no need to check for that.
    • Using StorageCapacity.purchases to determine how much Flow to return, withdraw from StorageCapacity.FlowVault edit StorageCapacity.purchases and reduce StorageCapacity.storageCapacity
    • return the withdrawn vault

Issue To Be Solved

Some users want to be able to query an account address to see if they operate a node

Suggest A Solution

• Add a public interface NodeInfoPublic that exposes the nodeID for a node operator

• NodeStaker should implement this interface

• Add a public interface DelegatorInfoPublic that exposes the nodeID and delegatorID or a delegator

• NodeDelegator should implement this interface

• Change transactions to create this public link when stakers are registered

• Create Script to read this node info from a PublicAccount

• Tests to make sure it all works

• Document it all

Issue To Be Solved

There are various improvements we could make to the staking and locked tokens contracts. This is to track those.

Suggest A Solution

• Add a total committed stake to NodeInfo object
• Add total tokens overall to NodeInfo object
• Add cancel unstaking request functionality
• Add a public interface to show an account’s node info

Issue To Be Solved

We current have a go package for getting transaction templates, but we should also have a javascript package so users of the contracts can import and use the correct versions without having to copy and paste.

Suggest A Solution

• Create a lib/go/javascript/templates package that does a similar thing to the lib/go/templates package.
• It should use the transactions and scripts in transactions/ as a source of truth, replace the import addresses with user provided addresses, and return the correct transaction texts.

Context

Would like to get this implemented soon because more organizations are using the transactions and scripts.

Issue To Be Solved

For epoch switchovers, we need to be able to perform all the epoch switchover operations in a single transaction so that new accounts cannot register in-between staking auction end and epoch end.

Suggest A Solution

• Write a single transaction that combines end_staking_auction.cdc, pay_rewards.cdc, and move_tokens.cdc
• Test the transaction

Issue To Be Solved

Need to change the storage fee contract to use the normal FlowToken Vault to track how much storage an account is allowed to use.

Suggest A Solution

Remove everything from the contract except for the conversion field, minimum field, and admin resource to change those fields.
Set Up tests and templates to test the contract
Change the service account to use the normal FlowToken Vault instead of the special Storage Fee vault

Need to design and code a first draft of the staking helper contract

Requirements are outlined in this doc: https://www.notion.so/dapperlabs/Token-Staking-Process-6877e85a057e4eab8beae7f5187f3ffb

Completion Criteria

• Conversation with Max, Josh, Layne, and Peter about staking helper requirements
• Clear high-level documentation in the notion document
• Updated flow chart
• First draft of staking helper contract that aligns with the design document

Issue To Be Solved

Add functionality to cancel a pending unstake request

Suggest A Solution

• Add functionality to the stake tokens functions to automatically cancel an unstake request if one exists
• Add tests for all three node functions and all three delegator functions
• Document how this is done in the docs and the FAQ

Suggest A Solution

• Go through the contract and find cases where unintended behavior could happen and make sure it behaves properly
• Nodes submitting transactions during the wrong phases
• No collector or consensus nodes
• Different mixtures of nodes
• resetting during all the phases
• transferring qc or dkg object to a new machine account and changing that in the staking collection
• Creating a machine account for a node that was added to the collection before the upgrade for epochs happened
• Find more

Issue To Be Solved

There are some functions that get data about nodes and delegators that should be moved to their respective Info structs.

Suggest A Solution

• getNodeCommittedBalanceWithoutDelegators

• getNodeCommittedBalanceWithDelegators

• getNodeStakedBalanceWithDelegators

• getOnlyDelegatedBalance

• Test the new cases

• Document these new cases

Problem

An error is thrown by Cadence when attempting to register a new node with the central staking contract via the LockedTokens contract.

Steps to Reproduce

1. Create an unlocked/locked account pair on testnet
2. Fund the locked account with 100.0 FLOW
3. Submit the Register Node transaction with amount set to 50.0
4. Observe the following error:

Execution failed:
  pre-condition failed: Amount withdrawn must be less than or equal than the balance of the Vault

Issue To Be Solved

Users with unlocked FLOW and locked FLOW want to be able to have a universal interface they can go through to stake their flow to multiple nodes/delegators in their account

Suggest A Solution

• Create a staking collection object that has capabilities to a user's staking objects
• Access the staking capabilities via the StakingProxy interfaces so that this staking collection can stake locked or unlocked tokens in the exact same way.

Context

Blocking ledger and flow port support for staking with unlocked FLOW

Issue To Be Solved

The staking reward needs to now be a proportion of all the tokens staked instead of the tokens staked for the node role.

(Optional): Suggest A Solution

• New Reward(user) = Tr * (Sn / St)

• Test with the correct delegation cut: 8%

Context:

Need to document events in the LockedTokens and Staking Proxy contract

Definition of Done:

Write an events page in the token docs site

Issue To Be Solved

With realistic node operator and delegator numbers (400 nodes, 10k delegators), iterating through all of them will break the gas limit by A LOT

Suggest A Solution

• Test staking and delegating with 500 Nodes and 20k delegators
• Measure gas usage for all normal staking transactions, scripts, and admin transactions (end staking, pay rewards, move tokens)

Context

When the DKG fails, it returns a key vector containing nil keys. Despite the failure, these key vectors should still be submitted to the smart contract. When deciding whether the DKG has completed, we should consider a key vector (final submission) containing any nil keys to be invalid, even if a majority of participants have submitted that key vector.

Definition of Done

• Decide how to serialize a nil key (either change smart contract to accept Array<Optional<String>> and encode nil keys as nil or define a canonical serialization for a nil key (eg. 0x00...) coordinate with https://github.com/dapperlabs/flow-go/issues/5452
• Update key vector submission / storage to use updated encoding, if necessary
• Modify logic determining when DKG is done to consider case where majority key vector contains nil keys as the DKG not being done -- this means the DKG has failed. Eventually we may want to take action when this happens, for now we just never say the DKG is completed and never emit the EpochCommit event

cc @danuio @tarakby @studio1vc

A balance of 10K Flow looks like this: balance: 1000000000000

Just checking that the minimums are right here:

Context

We must see what the time to execute and gas usage is for 1000 nodes and 20k delegators on testnet

Definition of Done

• Use the new changes and tests in lib/go/test/flow_idtable_nodes_test.go with 1000 nodes and 20k delegators on testnet
• Measure gas usage
• Measure how long it takes to execute the end_epoch and pay_rewards.cdc transactions with all these registered users.

PR Review

Who is your Code Partner - @Kay-Zee
Who can provide external eyes - @psiemens

Issue To Be Solved

Need to be able to verify that an account address has registered with the token admin before we send tokens to them, which implies that they have a correctly set up locked tokens account.

Suggest A Solution

Transaction:

• Signed by the token admin
• Check the token admin object and make sure the address is registered correctly
• Emit an event or panic if they aren't registered correctly

Issue To Be Solved

The locked tokens transfer needs to make sure that the address it is sending to has been registered

Suggest A Solution

Pre-condition in the deposit locked tokens function that makes sure it is registered in the admin collection

Context

• See image above for current situation and the problem we are facing.

Definition of Done

Issue To Be Solved

QC contract has been written but needs more thorough tests

(Optional): Suggest A Solution

• Expand happy path tests to include all edge cases
• Write more transactions
• Document transactions

Issue To Be Solved

For the intermediate stage of epochs before BFT is implemented, admin needs to be able to remove unapproved nodes before the staking auction ends

Suggest A Solution

• Work from feature/epochs branch
• Create new function similar to endStakingAuction that just removes unapproved nodes and doesn't end the auction.
• Potentially have this still end the auction. Need to investigate

Need a service account smart contract to manage delegation and enforce rules for node operators.

Details are in https://www.notion.so/dapperlabs/Flow-Delegation-a39af01f4ccc4ce3abaf7614679b3824

Issue To Be Solved

User's node Info should be able to be queried from a script by using their address.

Suggest A Solution

• Nodes: Add an interface that just includes the node ID
• Delegators: Add an interface that just includes the node ID and delegator ID
• Update the normal registration transactions to publish the capabilities
• Add the scripts to use these capabilities for querying node Info

Issue To Be Solved

This would allow third parties to securely and reliably receive and manage locked tokens on a user's behalf.

Proposed Solution(s)

Account Setup should have an option for the user agent to create only the User Account account 1 and the ability to request the creation of the Shared Account account 2 by Dapper Labs. This ensures Flow can safely and securely enforce token lock-ups without blocking community wallets from supporting people who hold locked Flow.

Add the ability for a user agent to request the flow token admin (or some other Dapper Labs controlled account) creates account 2 (so that Dapper Labs does not have to trust or check that account 2 was created correctly by the user agent.

Following initial account creation, the process and interactions should be identical. The user and custody/wallet provider's roles, abilities, and interactions should be the same.

• Details of the technical implementation @psiemens has given some thought to various options

@psiemens options:

1. Custodian is permitted to create the locked account, but only through a special method: FlowServiceAccount.createLockedAccount . This method would simply return the LockedTokens resource which the custodian can link into the unlocked account. We then know that the locked account was created properly
2. Custodian is not permitted to create the locked account and instead places a LockedTokenReceiver in the unlocked account. We then create the locked account at the time of token delivery and connect it to the unlocked account with the receiver.
3. The locked account is created automatically by the protocol when createAccount (AuthAccount(…)) is called. Every account receives a locked sidecar account by default

Caveats and considerations for the future

Option 1 considerations:
Currently custodians have account creator capability
Anyone unregisteered locked account is un-trusted
We could modify service account to have account creator capability - would return token manager resource which could be linked into public account - input would be users pubkey
tl;dr service account is account creator via pub.fun calling restricted code

Smart contract changes needed:

• a pub.fun doesnt require giving custodians their own capability - they could just call pub.fun --> reduces contract/integration load
• This will not remove earlier methods for creating locked account but will be an additional integration option

Other changes needed:

• Write updated transactions
• Write contract changes
• Document this alternative account creation process
• Re-deploy service account code (adding pub.fun for creating account)

Option 2 considerations:
Complicates matters to not have both accounts exist immediately

Option 3 considerations:
Not all accounts need to receive locked tokens and this would create hidden complexity

Reference Documentation: https://docs.onflow.org/token/staking/locked/setup/

Context

This would ensure the Blocto wallet could participate in supporting the initial token delivery.

Issue To Be Solved

Loading contracts for a script or transaction also has to load all the dictionary field values, so they need to be changed to use account storage instead.

Suggest A Solution

Define paths to store the fields for addresses and keys used instead of fields
define path to store staking enabled field as well
Define utility functions to access these paths in storage

Issue To Be Solved

We want token holders to have the option to delegate their tokens to multiple nodes should they choose to.

(Optional): Context

This address the missing functionality identified in the current documentation :https://docs.onflow.org/token/staking/locked/delegators/
Reference text:

Note: This transaction delegates additional tokens to the same node that was registered in the setup phase. It is currently not possible to delegate to multiple nodes from the same account.

Issue To Be Solved

The storage fees smart contract overflows when the account balance times the MbPerFlow constant is more than max UFix64. When it overflows it results in the account storage capacity being 0.

Instead it should result in the account storage capacity being max UFix64.

This is not a problem for mainnet as the entire FLOW supply is not high enough.

Context

When a node operator registers to stake, we should validate their public keys to ensure they are well-formed, using the updated Crypto contract in Cadence (onflow/cadence#582).

Definition of Done

• Staking key (BLS) and networking key (ECDSA) inputs are validated during staking registration
• A staking request with any invalid keys is rejected
• Public key fields are stored as the appropriate Crypto.PublicKey type

Context

Currently, we enforce that node info must be unique for each node, by iterating through the whole table and checking. This works, but is inefficient and uses a lot of gas, which could hurt normal users.

Definition of Done

• Add mapping fields for networkingAddress: Bool, networkingKey: Bool, stakingKey: Bool to indicate if a key or address has been registered already.
• Check these fields when a node registers.
• Write scripts to query these fields

Tests Required

• Make sure data is reserved when a node registers
• Check to see that gas usage is not affected with many nodes

Context

When the DKG fails, it returns a key vector containing nil keys. Despite the failure, these key vectors should still be submitted to the smart contract. When deciding whether the DKG has completed, we should consider a key vector (final submission) containing any nil keys to be invalid, even if a majority of participants have submitted that key vector.

Definition of Done

• Decide how to serialize a nil key (either change smart contract to accept Array<Optional<String>> and encode nil keys as nil or define a canonical serialization for a nil key (eg. 0x00...) coordinate with https://github.com/dapperlabs/flow-go/issues/5452
• Update key vector submission / storage to use updated encoding, if necessary
• Modify logic determining when DKG is done to consider case where majority key vector contains nil keys as the DKG not being done

cc @tarakby @danuio

Issue To Be Solved

Needs docs in the locked tokens documentation to describe new capabilities with locked tokens

Suggest A Solution

• Describe that users can now have a stake position and a delegation position at the same time
• Describe how to register a new staker only if all the tokens from the previous staker have been withdrawn
• Describe how to register a new delegator or it all the tokens from the previous delegator have been withdrawn

Issue To Be Solved

Currently the LockedTokens.LockedAccountInfo interface includes these methods:

• getNodeID - node ID of a staked node
• getDelegatorID - delegator ID

However, in order to fetch delegator info from the central staking contract, we need both the delegator ID and the node ID being delegated to. The currently implementation doesn't provide the node ID that is being delegated to.

Suggest A Solution

Add a getDelegatorNodeID function to the LockedTokens.LockedAccountInfo interface

Context

This is currently blocking @JeffreyDoyle from testing delegation through Flow Port

Context:

We need a smart contract that will manage T-system's rewards.

Definition of Done:

@laynelafrance to provide specs

Need to deploy contracts/FlowIDTableStaking on testnet and test with automated tests as well as manual tests.

Context

The current behaviour of the FlowDKG and EpochLifecycle contracts is to stop the DKG for a given epoch once the threshold (50%) of key vector submissions is met. We can still consider the FlowDKG complete once this threshold is reached, and emit the EpochCommit event at this point, but we would like to continue allowing key vector submissions after the threshold has been met.

This allows us to have a better sense for the health of the DKG by seeing how many nodes submitted valid key vectors (and therefore can be reasonably expected to participate in consensus in the next epoch).

Definition of Done

• EpochLifecycle - end DKG immediately before transitioning to next epoch rather than when the DKG reports being "done"

LockedTokensDeposited is declared in LockedTokens but not emitted in the code. Is it meant to be emitted in deposit() or somewhere?

https://github.com/onflow/flow-core-contracts/blob/master/contracts/LockedTokens.cdc#L44

https://github.com/onflow/flow-core-contracts/blob/master/contracts/LockedTokens.cdc#L304

Based on the work that is done via ticket https://app.zenhub.com/workspaces/flow-5a2ad022cee40e25b0885f82/issues/dapperlabs/flow-internal/1467, we will need stored interactions that implement the transactions that were created via this ticket to ensure Flow Port can call these transactions successfully.

Write tests to test contract described in https://github.com/dapperlabs/flow-go/issues/4530