plugblockchain / plug-blockchain Goto Github PK

Migrated from: cennznet/plug-blockchain#62 (comment)

The script should should send continual extrinsics to a plug network under test.

It should add random variance to:

• the type of extrinsic (balance transfer, attestation, etc.)
• sender account
• transaction rate (TPS)

It should stop after significant non-responsiveness by the network which would indicate a failure or a target block height is reached by majority of the nodes.

A/C - script demonstrated working on local plug development network running 1.0.0-rc.1

Should make use of https://github.com/plugblockchain/plug-sdk-types

Fix the build warnings for the develop branch.

To make the docker image work with our deployment pipeline (k8s), we need ENTRYPOINT instead of COMMAND. There are also two conflicting Dockerfiles:

• ./Dockerfile <- this seems to be the active dev one
• ./scripts/docker/substrate.Dockerfile <- the above may get refactored into this file later (but I'm not sure what they will do)

Task list:

• Refactor dockerfile
• Update changelog

There's an issue with the latest rust nightly, this appears to affect substrate as well. This results in the following error when trying to build the node-template:
Rust WASM toolchain not installed, please install it!

Sort term solution provided by @Holygits to use an older rust build:

rustup toolchain uninstall nightly
rustup toolchain install nightly-2020-08-23
rustup target add wasm32-unknown-unknown --toolchain nightly-2020-08-23

Currently if there are different locks on an asset for different reasons, when withdrawing from that asset we check if the widthraw reason overlaps with the locks reason and if so we apply the lock amount as a limit. The logic for this behaviour is not quite clear and intuitive. We need to review why we need locks and how they should behave according to the latest requirements of a lockable currency and improve or correct this behaviour.

Referenced in paritytech/substrate#6328
and fixed in the upstream here: paritytech/substrate@0c42ced

Due to historic reasons the notion of 'free balance' in GA is a misnomer for the staking currency.
We should alter this API / naming to ensure it is fool-proof

Background

expectation:
"free balance": the freely spendable balance of an account
reality:
"free balance": the maybe spendable balance, need to check locks also

This is extremely misleading for dapps which have to be aware of this difference and internal modules
when dealing with the staked currency must be aware of this also.

The currency locks API is made especially for staking, so this misnomer is only true for the staked asset.
No other asset can have locks, so the 'free balance' really is the freely spendable balance.

Goals

Make the generic asset balance API clear and fool-proof
Maintain backwards compatibility with the current API

Proposal

internal module API changes: free_balance

1. StakingAssetCurrency and MultiCurrency when asset is the staking asset must return the truely free balance (free balance - locked amount)

2. cennznet/api.js
   Provide a new freeBalance rpc which will always return the truely free balance e.g.: api.rpc.genericAsset.freeBalance and encourage dapps to swtich.
   Maybe place warnings in cennznet/api.js code against using api.query.genericAsset.freeBalance directly

The previous solution to cennznet/cennznet#341 which was #163 had to be reverted after merging the latest upstream due to causing a conflict in impl of Decode for RuntimeDispatchInfo.
We need to find a different solution or adjust the current one with the latest changes.

Asset info for each asset should feature the existential deposit for that asset. Generic Asset, post any operations that would potentially leave dust asset for an account should update the account data in the account store and purge the insignificant asset if the policy of that operation allows it. The account would be removed from the account store as well when there is no significant assets left.
A migration logic would be needed so the current storage of the accounts is searched for dust balances.
Endowed accounts defined in the genesis will be exempt from the dust balance check. Or in other worlds they should persist anyway.

Pipeline for release refs/tags/v2.0.1 failed. Please investigate.

If the pipeline has failed before pushing to crates.io, delete the release tag
and fix the release as necessary, retagging after complete. If the pipeline has
failed after pushing to crates.io, create a new tag incrementing the version.

The root cause of the issue reported here for cennznet seems to be in Plug/frame/system. In a test, after reducing the set_code weight to half, the runtime upgrade went through successfully.

Backport the corresponding fix from 1.0.0-rc4

Refactor error codes in PlugDoughnut with the decl_error! macro

Here:

Reference usage in srml/system:

Pipeline often fails with timeouts ~2 hours on the test step.

For comparison a clean local build takes around 35mins and a test ~40mins (requires building and executing tests)

Running the test suite in 1 hour seems like a reasonable target

Some options to improve this:

- Upgrade circleci plan with more CPU allocation
- Switch CI altogether to get more CPU (parity uses gitlab)
- Optimize caching with sccache
- Chunk tests by crates. we run them all in one step cargo test --all at the moment

Update GA module with a new storage map key: asset ID → value: {symbol, ID, dp}

Required fields:

• symbol (characters: A-Z, length: 8) *ignore uniqueness for now, assets are created by governance, unlikely to allow duplicates
• ID
• decimal places (hardcode to 4dp for now, as customising this has other implications)

GA create call should take this metadata also

Update node info and binary name for Plug

See: cennznet/plug-blockchain#55 (comment)

Also:

• Update the binary and package name to plug
  

  plug-blockchain/Cargo.toml

  Line 2 in 3d533cf

  name = "substrate"

  
  

  plug-blockchain/Cargo.toml

  Line 6 in 3d533cf

  name = "substrate"

• Update Dockerfile
  

  plug-blockchain/Dockerfile

  Lines 39 to 55 in 3d533cf

  	useradd -m -u 1000 -U -s /bin/sh -d /substrate substrate
  	COPY --from=builder /substrate/target/$PROFILE/substrate /usr/local/bin
  	# checks
  	RUN ldd /usr/local/bin/substrate && \
  	/usr/local/bin/substrate --version
  	# Shrinking
  	RUN rm -rf /usr/lib/python* && \
  	rm -rf /usr/bin /usr/sbin /usr/share/man
  	USER substrate
  	EXPOSE 30333 9933 9944
  	VOLUME ["/data"]
  	CMD ["/usr/local/bin/substrate"]

• "plug-node"
  

  plug-blockchain/node/runtime/src/lib.rs

  Line 80 in c337602

  impl_name: create_runtime_str!("substrate-node"),

Maybe add some sweet ascii art too?

________________/\\\\\\_______/\\\________________        
 _______________\////\\\______/\\/\\_______________       
  ___/\\\\\\\\\_____\/\\\_____/\\\//\\___/\\\\\\\\__      
   __/\\\/////\\\____\/\\\____\//__\//___/\\\////\\\_     
    _\/\\\\\\\\\\_____\/\\\______________\//\\\\\\\\\_    
     _\/\\\//////______\/\\\_______________\///////\\\_   
      _\/\\\____________\/\\\_______________/\\_____\\\_  
       _\/\\\__________/\\\\\\\\\___________\//\\\\\\\\__ 
        _\///__________\/////////_____________\////////___

Provide the means to query the extrinsic sender address during nested contract execution.
So that the extrinsic origin is known throughout nested contract calls.

For reference, caller() gives the address of the calling account (extrinsic sender at the top level, changes to calling contract for nested calls)

unblocks: #32

It should launch a new network of 5 validator nodes

• A/C add documentation (wiki, confluence, markdown) describing the charts flow
• Investigate re-purposing substrate helm chart

Managing secret keys should be left for a follow up PR if it is too difficult

See https://bitbucket.org/centralitydev/cennznet-node.helm.chart/src/master/ for inspiration

Refactor the circleci config to use the docker executor with the xlarge resource_class

Related to #7

Implement the hook from #32 for contract to contract calls.

• Add doughnut field to the contract ExecutionContext
  

  plug-blockchain/srml/contracts/src/exec.rs

  Line 263 in b93e358

  pub struct ExecutionContext<'a, T: Trait + 'a, V, L> {

• Expose execution context doughnut via new doughnut() method on the Ext trait
  

  plug-blockchain/srml/contracts/src/exec.rs

  Line 93 in b93e358

  pub trait Ext {

• Add contract-to-contract hook using Ext::doughnut()
  

  plug-blockchain/srml/contracts/src/wasm/runtime.rs

  Line 385 in b93e358

  let callee: <<E as Ext>::T as system::Trait>::AccountId =

With the current implementation of providing accounts in generic asset, even when an account is removed from the generic asset storage, generic asset keeps to be providing that account from the frame system's point of view. That's because reaping an account and thus its nonce from the account store would incur some complexity for the case the account is again created. As this may be a potential storage bloating to keep the dead accounts in the frame system, we should consider to remove them in a safe way later.
Trace the todo comments in the code with this issue number 191 as an initial step.

Generic Asset should allow correcting the previously-stored balances for a particular asset id. This can only happen on a runtime upgrade and based on the genesis config.

When used to provide AccountData to system pallet generic asset will always act as a "provider" for the account, even after all it's balance related storage has been reclaimed.

This prevents the system module from reclaiming the account storage e.g. current nonce.
For now this is the safest behaviour as removing an account's nonce has it's own set of challenges.

Issue opened here to track the behaviour.

warning: unused import: contracts_rpc_runtime_api::ContractExecResult
--> node/runtime/src/lib.rs:52:5
|
52 | use contracts_rpc_runtime_api::ContractExecResult;
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
|
= note: #[warn(unused_imports)] on by default

warning: unused import: StorageMap
--> core/test-runtime/src/system.rs:22:38
|
22 | use runtime_support::storage::{self, StorageMap};
| ^^^^^^^^^^
|
= note: #[warn(unused_imports)] on by default

Setup the circleci integration for this repo and plugblockchain org
The circleci config already exists

Add peerset logging improvements from invulnerables hotfix into develop

Substrate 2.0 is close here
It is a good chance to update larger components of this codebase now that the development cadence is slower upstream and hopefully more commitment to API stability.

In general the plug runtime has diverged from substrate and it does not need to follow the runtime changes upstream as they're mostly fed by different network or business requirements.
We do however want to follow the core framework where possible:

Things to pull in (in order of least difficulty):

• atomic transcations

• benchmark based weights
  we'd need to update to the new weight system + update benchmarking code

• consensus (babe + grandpa)
  Update to the latest

Take everything in client/ metric reporting, db backend updates, refactors the lot

• [general client/ update]

It's not clear if this change was backwards compatible.
e.g. new message types added, will need some careful testing to support current networks.

• [networking/lib-p2p update]

Plug node is inoperable with the flaming fir testnet so all references should be removed.

e.g.

The spike/2.0 branch is basically ready, still needs:

TODO:

• update prml/validator-manager
• update prml/consortium-permission
• update wiki/docs including doughnut architecture + Option signed extension type

Preparational step for releasing CENNZnet.

Currently when a lock is placed for an account, it will be used against all the assets (different asset ids) of that account which is wrong. The locks should be keyed by the asset ids and checked against the same asset id for which they are placed.
We may then need to implement lockable currency not for the whole generic asset module, but for a currency that is maintained by generic asset. Currencies like spending asset or staking asset and so on.

Bringing in this upstream fix: paritytech/substrate#6792

cennznet/cennznet#269

Bring the following changes from the upstream:

17/07/20 grandpa: report equivocations with unsigned extrinsics (#6656)

12/05/20 grandpa: missing equivocation reporting nits (#5953)

07/05/20 grandpa: report equivocations (#3868)

Updating block gas spent should be done independently from emptying gas meter, so if the latter function is overridden, there would be no need to deal with block gas spent too. This will also eliminate the need for exposing the block gas handling function.

As a node operator I want better logging from peer networking so that I can diagnose network issues easily

Tasks:

• Add logs for reputational changes and causes to peerset / other relevant modules (e.g. networking)
• PR made to <hotfix> branch also

Note: develop and hotfix are quite different. May need replicate the changes

As a node operator I want some network peers to be invulnerable to the reputation system so that my network is less volatile.

Tasks:

• Make a branch from cennznet/plug-blockchain 9d72ac28731d2dbf558f21683d17bd4f799213ca stable for the hotfix work

• Change on_report_peer behavior so that reserved node connections are not dropped
  

  plug-blockchain/core/peerset/src/lib.rs

  Line 261 in 9d72ac2

  if peer.reputation() < BANNED_THRESHOLD {

• Write integration tests to mock CENNZnet Rimu network structure
  https://github.com/plugblockchain/plug-blockchain/tree/develop/core/network/src/test

• A/C Hotfix deployed by SREs into CENNZnet Rimu network

• Refactor Plug to use the new versioned Doughnut enum type
• May require creating and tagging a new release branch for doughnut-rs (includes Cargo.toml version bump)

This should be part of the Plug and CENNZnet 1.0.0-rc2 releases

Provide initial trait and hooks for smart contract permissioning

• Augment the DelegatedDispatchVerifier trait
  It should provide functions to handle the following call scenarios:

• runtime calls contract
  verify_runtime_to_contract_dispatch(caller, doughnut, contract address) -> Result

• contract calls contract
  verify_contract_to_contract_dispatch(caller, doughnut, contract address) -> Result

• Implement the hook for runtime to contract dispatch (<T as system::Trait>::DelegatedDispatchVerifier>::verify_runtime_to_contract_call)
  

  plug-blockchain/srml/contracts/src/lib.rs

  Lines 570 to 579 in b93e358

  	pub fn call(
  	origin,
  	dest: <T::Lookup as StaticLookup>::Source,
  	#[compact] value: BalanceOf<T>,
  	#[compact] gas_limit: Gas,
  	data: Vec<u8>
  	) -> Result {
  	let origin = ensure_signed(origin)?;
  	let dest = T::Lookup::lookup(dest)?;

Ideally, a single query should be enough to know the whole assets and their metadat.

Pipeline for release refs/tags/v2.0.0 failed. Please investigate.

If the pipeline has failed before pushing to crates.io, delete the release tag
and fix the release as necessary, retagging after complete. If the pipeline has
failed after pushing to crates.io, create a new tag incrementing the version.

The study of recent upstream changes suggests to me that our generic asset module which is replacing the balances pallet should use an account store when dealing with creating or removing accounts. An account store will be bound to StoredMap and its functionalities is already implemented by frame_system::Module. The generic asset should only call functions like insert or try_mutate_exists when appropriate.

When creating a GA, initial issuance should be the whole number of tokens to create and the total supply should handle the decimal places i.e

total_supply = initial_issuance x 10.pow(decimal_places)

this will be a much simpler API to use client side

while we're here, limit decimal places to <= 18 seems sensible

Finish #124

Enable the submission of equivocation reports by validators
https://github.com/paritytech/substrate/blob/596f377d49fede088e0c32791fb72f1343003b0d/frame/babe/src/equivocation.rs#L143-L156

Similar to frame/balances/src/tests_reentrancy.rs in https://github.com/paritytech/substrate/pull/8087/files, Add tests to GA that would prove the reentrance safety of the functions there.

https://centralitydev.atlassian.net/browse/BT-124
https://centralitydev.atlassian.net/browse/BT-131

[email protected] in prml/doughnut/Cargo.toml should be corrected to [email protected]