Description
In the lockup contract, a vesting schedule is able to be terminated by the NEAR Foundation if a NEAR Foundation AccountId is provided, with the remaining unvested balance to be withdrawn by the NEAR foundation. If the contract does not have enough balance to cover the unvested amount, it assumes that the difference is in the staking pool and attempts to withdraw that.

The contract uses the minimum of the remaining unvested amount and get_account_balance() to determine how much can be withdrawn by the NEAR Foundation. However, due to the requirements to maintain a minimum balance for storage fees, get_account_balance() subtracts the MIN_BALANCE_FOR_STORAGE from the actual account balance.

It is possible to reach the case where all amounts have been withdrawn from the staking pool and there is insufficient account balance to cover the remaining unvested amount and MIN_BALANCE_FOR_STORAGE . This case can be reached by terminating a contract before any amounts have been vested.

As a result the NEAR Foundation will not be able to withdraw the entire unvested balance. The contract will be stuck in the TerminationStatus::ReadyToWithdraw status, and get_terminated_unvested_balance() will be equal to the MIN_BALANCE_FOR_STORAGE .

To unlock the contract and withdraw the remaining balance there must be a deposit of MIN_BALANCE_FOR_STORAGE made to the contract.

Recommendations
Ensure that the development team is aware of this edge case which may require transferring an additional 35 NEAR to finalise termination of the contract.

When confirming a request on the new multisig contract #66 requests don't appear to be removed properly.

Steps to reproduce:

1. deploy the contract as 1/1 confirmations multisig
2. create a new request to change confirmations to 1 (trivial request)
3. confirm request
4. request still in storage via list_request_ids i.e. request id is returned in Vec
5. delete_request produces error here
6. confirming again produces error here
7. get_confirmations produces error here

Based on (7) it seems like the request is removed from confirmations, but based on (4) not removed from requests.

Currently looking into this. @ilblackdragon might be able to spot why a bit faster.

Currently our tooling only knows to look for lockup contracts under *.lockup.near

But this means that only NF can deploy those.

This can be easily made permissionless by creating a factory contract similar to staking factory to deploy lockups and deploy that to lockup.near account.

Description

NEAR Protocol uses human-readable account names. This allows for much better user experience for regular blockchain use cases. We also expect that these accounts will be used across various novel use cases (e.g., DNS, certification, and applications) as they represent the user’s digital identity that is also connected to finances, data ownership, and cryptography—and, in the future, supporting more of Decentralized Identifier (DID) standard.

We split account names into two groups by length: those longer than 32 characters and those shorter. We expect that shorter accounts names will have way more value for users compared to longer ones. Longer account names can register on a first-come-first-serve basis.

To give everyone a fair chance to acquire short account names, a naming auction will start shortly after MainNet launch.

Each week’s account names—such that sha256(account_id) % 52 is equal to the week since the launch of the auction—will open for bidding. Auctions will run for seven days after the first bid, and anyone can bid for a given name. A bid consists of a bid and mask, allowing the bidder to hide the amount that they are bidding. After the seven days run out, participants must reveal their bid and mask within the next seven days. The winner of the auction pays the second-largest price.

Proceeds of the auctions then get burned by the naming contract, benefiting all the token holders.

Spec

API

struct Bid {
  amount: Balance,
  commitment: Base58Hash
}

struct Auction {
   start_blockheight: BlockHeight,
   bids: Map<AccountId, Bid>,
   reveals: Vec<AccountId, Balance>,
}

struct Registrar {
  start_blockheight: BlockHeight,
  auctions: Map<AccountId, Auction>
}

impl Registrar {
   /// Construct this contract and record starting block height.
   pub fn new(auction_period: BlockHeight, reveal_period: BlockHeight) -> Self;
   /// Attached deposit serves as locking funds for given account name.
   /// Commitment is `hash(masked amount + salt)` in base58 encoding.
   /// bid fails if `account_id` is not yet on the market based on `hash(account_id) % 52 > weeks from start_blockhegiht`
   /// bid records a new auction if auction for this name doesn't exist yet.
   /// bid fails if auction period expired.
   pub fn bid(account_id: AccountId, commitment: Base58Hash);

   /// Reveal shows the masked amount and salt. Invalid reveals are declined.
   /// Reveal fails if auction is still going.
   /// Reveal fails if `hash(masked_amount + salt)` != `commitment` by env::predeccessor_account_id()`
   pub fn reveal(account_id: AccountId, masked_amount: U128, salt: String);

   /// Withdraw funds for loosing bids.
   /// Withdraw fails if account_id doesn't exist, if `env::predeccessor_account_id()` didn't bid or if auction is still in progress or not all bids were revealed yet.
   /// If not all bids were revealed but required reveal period passed, can withdraw.
   pub fn withdraw(account_id: AccountId);

   /// Creates the new name with given public key for the winer.
   pub fn claim(account_id: AccountId, public_key: Base58PublicKey);
}

State machine

Name auction can be in the next states:

• Locked: this name is not yet ready for auction
• Unlocked: the auction can be created for this name, transition from Locked after block_height > sha256(name) % 52 * blocks_per_week
• Open: auction has started, to transition anyone need to call bid on Unlocked auction name. Accepts more bid transactions.
• Reveal: after X blocks from auction opening, automatically switch to reveal state and accept reveal transactions.
• Claiming: reveal period has expired after X blocks or all bids were revealed. Accepts claim for this name. Allows to withdraw all bids that have not won.
• Done: account was claimed and created, the auction is done and all state will be cleared except that this name is in done collection. On claim also withdraws all other bids automatically.

This repo and thereby the contracts are missing a LICENSE or UNLICENSE file. This usually becomes a bigger problem the longer one waits (more contributors, etc.)

Impact: High
Likelihood: Low
Reported by a third-party.

Description
In the lockup contract, main public keys are used by the owner to operate certain owner functions such as adding and removing other keys and transferring funds.

The current owner is able to remove all keys including their own key, leaving the contract with no remaining main keys. This would effectively lock any remaining balance in the contract.

An accidental case where this occurs would be when a user sends two transactions, one to add a new key and the other to remove the existing key. If these two transactions are executed in the wrong order (i.e. the deletion first). The contract will be locked.

Recommendations
To add a level of protection to accidental deletion it is recommended to either:
• Prevent deletion of the signing key;
• Ensure there is at least one main key in the list of keys

Help users who want to deploy their staking pool via factory by providing examples of command-line based deployment.
Would be great also to change the naming from stakehouse to stakewars 😜

When a staking pool is created, there is no check on whether the owner account exists. Since staking pool is usually locked (when created from the factory for example), the owner cannot do anything but abandon this staking pool if this happens.

Currently we have 2 methods defined to delegate stake with a validator.

near call my_validator deposit '{}' --accountId user1 --amount 100
near call my_validator stake '{"amount": "100000000000000000000000000"}' --accountId user1

https://github.com/near/initial-contracts/blob/master/staking-pool/README.md#usage

A state machine of the contract:

Problem:
Wallets must authorize 2 txs to stake with a validator. Often a user ("delegator") will want to stake the amount they deposit immediately.

Solution:
I propose a new payable method deposit_stake that combines the functionality of the above 2 methods into 1.

As was pointed out to me by @mikedotexe, could this be solved using batch transactions? I am unfamiliar with how these work.
https://github.com/near/near-sdk-rs/blob/add801738404a74813e74ed3e0447e9e64805c3a/examples/cross-contract-low-level/src/lib.rs#L17-L28

Right now if I want to verify the user doesn't have any balance on the staking pool, I have to call both get_user_balance and get_user_stake

We currently implicitly expect people to know how to build smart contracts in Rust for NEAR, and then they can hack around this code-base. I believe we should point people to our tooling and getting started documentation as well as describing the flow on how to build a bit-by-bit identical binary to those that we deploy, and thus confirming that our binaries are built from the source code.

Action Items

• Add links to getting started documentation, so people know how to build and how to work with the contracts (rustup run stable rustup target add wasm32-unknown-unknown)
• (optionally) Improve the build scripts to include the check for the wasm toolchain being available
• Describe bit-by-bit reproducible builds

/cc @nearmax

I suggest to add the following methods to the lockup contract:

• transfer_to_exchange

A method to be able to transfer directly to an exchange. It's similar to transfer method, but uses a function call with a given memo. See https://github.com/near/core-contracts/tree/master/exchange-deposit-receiver

• update_staking_pool_balance

When delegating to a staking pool from a lockup contract, the lockup contract keeps track of the amount of tokens transferred to the staking pool. It's stored internally as a deposit_amount and can be queried using view method get_known_deposited_balance().

Since the delegated balance in a staking pool can acquire rewards due to staking rewards, the deposit_amount can grow. But there is no way to refresh the deposit_amount without unstaking and withdrawing all tokens from the staking pool.

This method should query the staking pool contract update internal deposit_amount balance.

• deposit_and_stake

It's a helper method that is now supported by the staking pool to avoid calling deposit and stake as 2 separate function calls.

• unstake_all

Allows to unstake everything from the staking pool.

• withdraw_all

Allows to withdraw everything from the staking pool.

When multiple validators are deployed it is difficult to know which contract is deployed to each. A convention to force a name / version on contracts would be very helpful when contracts need to be upgraded or migrated to new ones.

Hi,

Due to taxation issues/situations in some jurisdiction it would be very helpful if you could have the option to pay out the staking rewards to a separate address instead of always automatically re-staking them.

Context: as a user, I would like to know which version of the Staking Pool contract is being used, to simplify the audit of a validator before sending my tokens.
Ideally, I should reveal this information by using a near view command.

Alternatively, knowing the hash/fingerprint of the deployed contract, and having an easy way to find its original source code on Github will do the job.

We need to create a docker image (Docker file is enough, no need to publish it) that people can use to build these contracts. This Docker image will have fully fixed versions of the toolchain which will guarantee 100% reproducibility of the Wasm files that it outputs given the source code. This will allow community members to verify that the Wasm files that were deployed were in fact compiled from the provided source code.

Current ping version is going to hit the gas limit for storage_write limits quite fast. It's likely the limit is for 20-30 users max

Informational
Reported by a third party.

Description
The functions transfer() and transfer_from() send an amount of tokens from one account to another. In both cases it is valid to transfer the tokens to and from the same account.

In transfer_from() this can be done by setting owner_id to be the same as new_owner_id .

In transfer() this can be done by setting new_owner_id to be the same as predecessor_account_id.

The net change in the accounts balance will be zero and thus there is no known attack vector using this approach.

Recommendations
It is recommended to ensure that owner_id and new_owner_id represent different accounts.

Currently there does not seem to be a way to see all users that have staked to the contract. This creates an issue when you need to unstake to upgrade the contract. For instance Stefano sent some NEAR to one of my contracts and I'm not sure how to unstake him.

When the contract does not have enough gas the error that is given to the user is not descriptive enough.

ERROR:
Scheduling a call: blaze_betanet.unstake({"amount":"10000000000000000000000000000"})
Error: GuestPanic [Error]: Smart contract panicked: panicked at 'The new total balance should not be less than the old total balance', src/lib.rs:507:9

Recommended ERROR: Out of gas, send more NEAR to the contract address.

The suggestion is to change lockup to have 0 access keys.

Instead have a master account that is specified on deployment.

• Lockup contract is only aware of it’s master account
• master account can have a multisig instead of full access key to improve security and allow key management
• master account can authorize staking UI for using lockup contract on it’s behalf of it with stake,deposit,withdraw,unstake authorized functions.

Meaning that all public methods inside lockup for user just have a guard of assert_eq!(env::predessesor(), self.owner_id); and methods for foundation have assert_eq!(env::predessesor(), self.foundation_id);

On the wallets / custody side, this would mean that they can just control master account. A separate page for lockup operations can be created, where users can "Login with NEAR" and then withdraw vested tokens. Staking of locked tokens can be done from this page as well (or separate staking app can be used, but needs to be aware of this dynamics).

We can create master accounts via linkdrops with multisig contract as default and only enough $NEAR to cover the contract storage + 3 keys.

CC @kcole16

Reported by a third-party.
Impact: Medium
Likelihood: Medium

Description
In the fungible-token contract, transferring tokens to a new account will insert a key value pair into the accounts map in FungibleToken . The key is a sha256 hash of the account id and a new Account object as the value. The Account object contains a Balance and another Map which is a key pair of a sha256 hash of an account id and a Balance .

This implies a large quantity of storage is required to create a single account. Accounts can be created from the transfer of a single token which may have negligible value depending on the usage. At the cost of the transaction gas and a single token, per account, the attacker can generate a large quantity of storage by creating numerous accounts. As a result the contract will face high storage fees.

Alternatively, an attacker can create numerous entries into the allowances map requiring only a single token to create practically infinite allowances. The result is similar in that a large quantity of storage will be used by the contract at the cost of the transaction gas, resulting in high storage fees for the contract.

Note that these accounts only need to have valid IDs (i.e. they do not need to actually exist on the Blockchain).

Recommendations
The mainnet gas price is currently set to 100, 000 yocto per gas which would make the cost ratio 1/(5 ∗ 104) implying this is still a cheap and feasible attack.
To mitigate this type of attack, the cost of creating a new account can be increased. This may occur by any of the following:
• The contract charging a fee for an account creation;
• The contract burning an additional quantity of gas when creating a new account;
• The contract only allowing insertions of new accounts that exist;
• The network increasing gas price charged;
• Reducing storage fee requirements (could have undesirable side effects).

Informational
Reported by a third party.

Description
In the fungible-token contract, FungibleToken::set_allowance() takes an allowance as input which
gives another account the ability to spend these funds on their behalf.

There are no checks to ensure the current account balance of the sender is greater than the allowance. This may lead a user to believe that they have an allowance larger than what is actually available.

Recommendations
This is the same behaviour as the Ethereum ERC20 token specification. As such no action is required other than the developers be aware of this behaviour.

When a near view command is called with an incorrect validator name the contract fails with error: "error": "wasm execution failed with error: FunctionCallError(MethodResolveError(MethodNotFound))",

When we do hard-forks, we ask validators who cannot be online during the hard fork to unstake, but there is no way for the owner to unstake all the stake.

It is also useful for maintenance purposes.

/cc @jimmy3dita @nearmax

Currently there is no way to view who are delegating to given staking pool.
Let's add list_delegators or similar method, that returns all users who have delegated with their number of shares.
This will allow to make explorers and other tools for delegation easier.

Informational
Reported by a third party.

Description
There are currently checks to validate the input accounts for get_allowance() . These checks prevent contracts accidentally checking the allowance of an invalid account.

The function get_balance() does not posses the same checks. Users may accidently check the balance of an invalid account. The return value of the call would be zero.

Furthermore, there is a comment for get_allowance() stating how the allowance may have already changed by the time the return value is read by a contract. The same principle also applies to get_balance() , however it is not documented.

Recommendations
While these checks and comments are not necessary, it is recommended to have a consistent behaviour across similar functions.

Since a lot of contracts depend on each other, we should implement CI to make sure one doesn't break another.

Informational
Reported by a third party.

This section details miscellaneous findings identified by the testing team that do not have a direct security implication:
fungible-token

• line [56] of fungible-token/src/lib.rs includes a comment with no message. (i.e. // ).
• It should be documented that the contract account should be keyless or there is the potential for modification by the owner.

staking-pool

• There is no limit to the owner’s reward percentage. They may modify it to 100% and claim all rewards.
• Any transfers made by a user (i.e. using send ) without a function call will be treated as rewards and distributed to stakers accordingly.
• If an account calls withdraw() and the same account is deleted before the transfer() promise
  has been executed, the transfer will fail but the changes to StakingContract will not be reverted.
  The result is that the fees will be distributed to the remaining stakers as rewards during the next
  internal_ping() .
• If a validator is slashed, the remaining funds will be locked in the contract until someone sends a
  sufficient balance for the contract to have total_balance >= last_total_balance .
• line [27]: /// A type to distinguish between a balance and a staking shares for better
  readability. -> and staking shares
• line [32]: The to do, // TODO: Revert back to 4 once wasm/wasmer bug is fixed. , may now
  be updated.
• line [72]: Which will not unlock the funds for 4 epochs. ->
  It will not unlock the funds for 4 epochs.
• line [146]: /// It prevents inflation the price of the share too much. ->
  /// It prevents inflation of the price of the share too much.
• line [214]: /// It’s only allowed if the ‘unstake‘ action was not performed in the recent
  3 epochs. -> in the 3 most recent epochs.
• line [275] & line [346]: Calculated staked amount be positive, ... ->
  Calculated staked amount must be positive, ...
• line [383]: near/nearcore#2636 , the associated PR has
  been merged and note can be removed.
• line [495]: // Checking if we need there are rewards to distribute. ->
  // Checking, if we need, there are rewards to distribute.
• line [520]: // Now buying "stake" shares for the contract owner at the new shares price.
  -> at the new share price.
• line [532]: // got any shares or not. -> has any shares or not.
• line [617]: /// Inner method to get the save the given account for a given account ID.
  -> Inner method to save the ...

lockup

• Type ProposalId is u64 as opposed to U64 .
• line [9] of types.rs : The to do, // TODO: Revert back to 4 once wasm/wasmer bug is fixed. ,
  may now be updated.
• line [1] of lib.rs : //! A smart contract that allows tokens lockup. ->
  allows tokens to be locked up. .
• line [50] of foundation.rs : env::panic(b"There are no termination in progress"); ->
  There is no termination in progress .
• line [69] of gas.rs :
  /// Gas attached to the inner callback for processing result of the unstake call to the
  has a double space after call.
• line [26] of owner_callback.rs :
  /// Called after a deposit amount was transferred out of this account to the staking pool
  is missing a full stop.
• line [48] of owners.rs :
  // staking pool, but it’s on the owner. The contract doesn’t care about leftover
  -> // staking pool, but it’s attributable to the owner. The contract doesn’t care about
  leftovers.

staking-pool-factory

• line [4] of README.md : It allows any user to create an new whitelisted staking pool. -> ...create a new ... .
• line [20] of README.md : Missing an extra slash in // contract .

Recommendations
Ensure that the comments are understood and acknowledged, and consider implementing the suggestions above.

This should become a macro.

Originally posted by @willemneal in #72

There was an incident where @Arn77 transferred 75k NEAR to the "contract address" via the web wallet and presently there is not a way to transfer that NEAR back to the transferring account or to another account.

Issue:

1. When using the transfer function on https://wallet.betanet.near.org "contract accounts" are showing up.
2. Once NEAR is transferred to a contract account, it should be able to be transferred back out.

What are the usecases for NEAR being transferred to a contract?

1. For gas?
   2 In the case the contract itself wants to participate in staking?

Impact: High
Likelihood: Medium
Reported by a third-party.

Description

There are two actions required to deploy and initialize a contract. The first is the deployment of the code which involves uploading the contract byte code ( .wasm file). The second is the initialization function (e.g. new() or default() ), which sets the account state storage based on the code in the contract. The initialization action may be called from any account.

If these actions are not part of a single transaction, it is possible to front-run the initialization transaction.

When an attacker witnesses a transaction to deploy the contract they may wait for the execution of the transaction, then front-run the initialization transaction (i.e. the call to new() ) with their own parameters. Front-running of transactions in this scenario is trivial for any block producer as they may decide the order of the transactions inserted into a new block and may always create the following scenario:

1. Deployment transaction;
2. Malicious initialisation transaction;
3. Owner initialisation transaction.

In the case of contract, an attacker can front-run the call to new() . This will allow the attacker to set the , the and the transfers to be enabled. The attacker may then transfer the entire balance of the contract to their personal account.

In the case of contract, an attacker can front-run the call to new() . This will allow the attacker to set the parameter, gaining ownership of the contract. The contract is required to be initialised with an initial balance which cannot be attached to the call to new() as it is not payable. The attacker will therefore receive all staking rewards fees associated with the contract, as they may set the reward percentage to 100%. Therefore, the fees received from this initial balance will be rewarded to the new owner.

In the case of the contract, the front runner can here again front-run the new() transaction call. The attacker may also set the parameter which will allocate them the entire token supply.

Recommendations
Ensure there is adequate documentation in all cases to warn users against the dangers of using multiple trans- actions. When deploying new contracts, developers should always initialize contracts in the same transaction.

This issue is related to PR #8

Sequence of events:

1. c1.nearkat deploys the staking pool contract on Betanet
2. nearkat deposit and stake 80,000 tokens to c1.nearkat
3. c1.nearkat becomes validator and generates rewards
4. c1.nearkat shuts down the node and gets kicked out from validators
5. c1.nearkat now owns 100% of the unstaked balance, and can transfer it

Proofs:

• transaction G1sm9s8iJe7v5AyTkH4ZjWgeNfLnBrRhkHUNeNUwfC3H

near-shell output:

$ near send c1.nearkat nearkat.betanet 80300
Using options: {
  networkId: 'betanet',
  nodeUrl: 'https://rpc.betanet.nearprotocol.com',
  contractName: undefined,
  walletUrl: 'https://wallet.betanet.nearprotocol.com',
  helperUrl: 'https://helper.betanet.nearprotocol.com',
  sender: 'c1.nearkat',
  receiver: 'nearkat.betanet',
  amount: '80300',
  initialBalance: null
}
Sending 80300 NEAR to nearkat.betanet from c1.nearkat
{
  status: { SuccessValue: '' },
  transaction: {
    signer_id: 'c1.nearkat',
    public_key: 'ed25519:BXPywqpaPC8pH9Zexdh2B1S6SsXug7kfp2jykEtRA3dg',
    nonce: 4,
    receiver_id: 'nearkat.betanet',
    actions: [ { Transfer: { deposit: '80300000000000000000000000000' } } ],
    signature: 'ed25519:3hUtE1ko6SekVETz46CUQsJCRw5w2WtjKTENUcJpACyYKqYJ9rm6FA2Nyz5ngeCt3Dxi4NWpHpQpbehX9U3aL75z',
    hash: 'G1sm9s8iJe7v5AyTkH4ZjWgeNfLnBrRhkHUNeNUwfC3H'
  },
  transaction_outcome: {
    proof: [],
    block_hash: '963FRbjDXF7TourieWBUAU96Veg3FxTccY5q4nM9r6av',
    id: 'G1sm9s8iJe7v5AyTkH4ZjWgeNfLnBrRhkHUNeNUwfC3H',
    outcome: {
      logs: [],
      receipt_ids: [ 'HvSatv2paC86AeLrGCw1BAZotmkto1SD9cMZiiLZkYxT' ],
      gas_burnt: 937144500000,
      status: {
        SuccessReceiptId: 'HvSatv2paC86AeLrGCw1BAZotmkto1SD9cMZiiLZkYxT'
      }
    }
  },
  receipts_outcome: [
    {
      proof: [],
      block_hash: 'AauckaMCxTcJfm1hLtEopewgFyPypk52i5drodnevHvo',
      id: 'HvSatv2paC86AeLrGCw1BAZotmkto1SD9cMZiiLZkYxT',
      outcome: {
        logs: [],
        receipt_ids: [],
        gas_burnt: 937144500000,
        status: { SuccessValue: '' }
      }
    }
  ]
}

I believe it would be helpful if each function were thoroughly commented. Each parameter should have clear descriptions, and significant sections of logic should have comments explaining what's happening.

A good example for contract interfaces: https://eips.ethereum.org/EIPS/eip-721

Because NEAR runtime doesn't have a way to query access keys that current account has, contract must have an internal set of keys. This would also allow to configure different options around specific keys.

/// Contain list of options that multisig supports for given key.
/// For example: no confirmation for some small amounts or whitelisted accounts.
struct KeyOption {
   ...
}

struct Multisig {
  ...
  keys: HashMap<Key, KeyOption>
}

impl Multisig {
  ..
  fn new(keys: Vec<Base58PublicKey>) -> Self {
       ... actually add keys to access keys ...
  }
  

Features:

• Addition or updating keys allows to set options
• Deletion of keys should prevent deleting keys below the number of confirmations (removes the current gotcha)

The vote method on the staking pool readme is not up to date, as it still requires the proposal_id as parameter:
https://github.com/near/core-contracts/tree/master/staking-pool#interface

The correct syntax should be
{"voting_account_id": <voting_contract_id>, "is_vote": true}

I've sent 500 $NEAR from an account with 500 $NEAR. It succeeded but now I can't anymore interact with the contract.
Deposit TX - https://explorer.betanet.nearprotocol.com/transactions/FjpmJWbdBfWbGctwpJrGQpEViYWGgm7rHKzL64WxXTXp

o:nearprotocol pepe$ near call nearkat withdraw '{"amount": "500000000000000000000000000"}' --accountId nuc2.test
Using options: {
  accountId: 'nuc2.test',
  networkId: 'betanet',
  nodeUrl: 'https://rpc.betanet.nearprotocol.com',
  contractName: 'nearkat',
  walletUrl: 'https://wallet.betanet.nearprotocol.com',
  helperUrl: 'https://helper.betanet.nearprotocol.com',
  gas: '100000000000000',
  amount: '0',
  methodName: 'withdraw',
  args: '{"amount": "500000000000000000000000000"}'
}
Scheduling a call: nearkat.withdraw({"amount": "500000000000000000000000000"})
Error:  GuestPanic [Error]: Smart contract panicked: panicked at 'attempt to divide by zero', src/lib.rs:103:47
    at Object.parseRpcError (/usr/local/lib/node_modules/near-shell/node_modules/near-api-js/lib/utils/rpc_errors.js:25:19)
    at Account.signAndSendTransaction (/usr/local/lib/node_modules/near-shell/node_modules/near-api-js/lib/account.js:114:36)
    at processTicksAndRejections (internal/process/task_queues.js:97:5)
    at async scheduleFunctionCall (/usr/local/lib/node_modules/near-shell/commands/call.js:29:34)
    at async Object.handler (/usr/local/lib/node_modules/near-shell/utils/exit-on-error.js:4:9) {
  type: 'GuestPanic',
  index: 0,
  panic_msg: "panicked at 'attempt to divide by zero', src/lib.rs:103:47"
}

Impact: Low
Likelihood: Low
Reported by a third-party.

Description
Front-running attacks [1, 2] involve users watching the blockchain for particular transactions and, upon observing such a transaction, submitting their own transactions to be executed before the original transaction.

Furthermore, front-running can simply be done by block producers by inserting their transaction before the original transaction when producing a block.

The front-running vulnerability exists in the set_allowance() function.
Consider the following scenario:

1. Alice approves Malory to spend 1000 tokens, by calling the set_allowance() function on the token contract;
2. Alice wants to reduce the allowance, from 1000 tokens to 500 tokens, and sends a new transaction to call the set_allowance() function, this time with allowance as 500 .
3. Malory watches the blockchain transaction pool and notices that Alice wants to decrease the allowance.
   Malory proceeds with sending a transaction to spend the 1000 tokens, which if mined before Alice’s second transaction;
4. Alice’s second set_allowance() happens, effectively providing Malory with an additional allowance of 500 tokens;
5. Malory spends the additional 500 token. As a result, Malory spent 1500 tokens, when Alice wanted her to only spend 500 tokens.

Recommendations
Be aware of the front-running issues in set_allowance() and potentially add extended allowance functions which are not vulnerable to the front-running vulnerability. The functions increase_allowance() and decrease_allowance() may be used to prevent double spending
of the allowance, however decrease will still be vulnerable to front-running the amount to be decreased. A further method typically used to address this attack vector is to force users to change the allowance to 0 first, before updating it to the desired value (requires two separate transactions). See the safeApprove() function in OpenZeppelin’s SafeERC20 contract.

Informational
Reported by a third party.

Description
The Rust release profile does not have overflow checks on by default. As a result, any overflows (e.g. addition, subtraction, etc.) will simply wrap, potentially creating an inconsistent state.

No scenarios could be found to trigger overflows in the fungible-token contract.

Recommendations
The release profile should be updated to panic on overflows, that is, to include overflow-checks = true .

Steps to reproduce:

1. deposit and stake tokens e.g. 100
2. deposit more tokens but do not stake them e.g. 50
3. withdraw tokens e.g. 25 works
4. now call unstake with any amount of tokens e.g. 1
5. cannot withdraw remaining amount of unstaked tokens e.g. 25 does not work

What was expected (based on example amounts above):
staked: 75
unstaked: 26
unavailable for withdrawal until unstaked_available_epoch_height: 1
available for immediate withdrawal: 25

What happened:
staked: 75
unstaked: 26
unavailable for withdrawal until unstaked_available_epoch_height: 26
available for immediate withdrawal: 0

I believe this is due to the unstaked balance being treated as a single contiguous balance of tokens with the restrictions imposed by unstaked_available_epoch_height of the Account.

Another example is receiving rewards and "taking profits" continuously. If you repeatedly call unstake before withdraw, you will have an unstaked balance that can never be withdrawn.

Why is this a bug? People should have access to funds as they are unstaked and past the available epoch for each unstaking.

Proposal:
Treat each unstaking as it's own balance with it's own unstaked_available_epoch_height. Offer a method to calculate unstaked_available_balance and unstaked_pending_balance that returns funds available for withdraw immediately and funds that are still waiting for the available epoch height respectively.

There are usually few pieces of data about the pool that people are interested in:

• Logo (IPFS hash or url)
• Description
• Website URL (or IPFS hash)

This optionally can leave in a separate registry too.

Let's return balance of 0 for both liquid and stake balances if the user doesn't exists.

https://github.com/near/staking-contract/blob/c9f463cfbf5c0d75d9656fde98789d34830ddd2e/src/lib.rs#L169-L178

Since near/nearcore#2805, if someone tries to stake less than the minimum threshold, the staking transaction will fail. This causes issues with the staking pool. For example, when some large delegator wants to withdraw all their stake, the rest of the stake left in the staking pool might be less than the minimum threshold, in which cause the unstaking will fail because it tries to restake with what is left and this causes the contract to enter an inconsistent state because from the contract perspective, unstaking succeeds. A similar issue exists if someone delegates a small amount of tokens to the pool such that the total stake in the pool is below the minimum threshold. We should check the result of the staking action to prevent the contract from entering such inconsistent state.

Today we had a hardfork and I found that all my delegated tokes are not locked in the staking pool. I didn't find it in the proposals list as well

{
  amount: '92041683339174666976069487414',
  locked: '0',
  code_hash: 'BtAc7jTx38AXSrfEP3cW9ByQ69VS7YD1vLm73AmhpLFu',
  storage_usage: 248974,
  storage_paid_at: 0,
  block_height: 6001655,
  block_hash: '4Y1SXDHbN6UVnXMkwV8Y32jjLjuvfhswbYzXd98CkFVm',
  formattedAmount: '92,041.683339174666976069487414'
}

I made new deposit / stake for 10 $NEAR and everything got back

{
  amount: '834000000007922716886770966',
  locked: '91217683339174665976069486448',
  code_hash: 'BtAc7jTx38AXSrfEP3cW9ByQ69VS7YD1vLm73AmhpLFu',
  storage_usage: 248974,
  storage_paid_at: 0,
  block_height: 6001902,
  block_hash: 'EawJi8WsmQ3LAA698jp5NPxW9p42wiZi5KqGpk3ovfav',
  formattedAmount: '834.000000007922716886770966'
}

Solution: ping contracts after hardforks

Now we have Staking / Delegation contract . already dont need near login (https://docs.near.org/docs/validator/staking) staking, right ?

If the staking pool is created on an account with balance = 1_000_000_000_000, then total_stake_shares is 0, and several view functions start to panic

Note: I'm preparing a PR to handle this edge case

To reproduce, add this unit test: (the 4th parameter is the account initial balance)

    #[test]
    fn test_low_staked_balance() {
        let mut emulator = Emulator::new(
            owner(),
            "KuTCtARNzxZQ3YvXDeLjx83FDqxv2SdQTSbiq876zR7".to_string(),
            zero_fee(),
            STAKE_SHARE_PRICE_GUARANTEE_FUND, //account initial balance, minimum, edge case
        );
        //bob deposits 1M
        let deposit_amount = ntoy(1_000_000);
        emulator.update_context(bob(), deposit_amount);
        emulator.contract.deposit();
        emulator.amount += deposit_amount;

        //bob wants to know how much unstaked balance he has
        emulator.update_context(bob(), 0);
        assert_eq!(
            emulator.contract.get_account_unstaked_balance(bob()).0,
            deposit_amount
        );
        //panic!
        //thread 'tests::test_low_staked_balance' panicked at 'The total number of stake shares can't be 0', src/internal.rs:297:9

    }