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

feat: Allow miner add an arbitrary message into the cellbase #1000

Merged
merged 3 commits into from
Jun 15, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions miner/src/block_assembler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,13 @@ use ckb_core::transaction::{
};
use ckb_core::uncle::UncleBlock;
use ckb_core::BlockNumber;
use ckb_core::{Bytes, Cycle, Version};
use ckb_core::{Cycle, Version};
use ckb_logger::{error, info};
use ckb_notify::NotifyController;
use ckb_shared::{shared::Shared, tx_pool::ProposedEntry};
use ckb_store::ChainStore;
use ckb_traits::ChainProvider;
use ckb_verification::TransactionError;
use crossbeam_channel::{self, select, Receiver, Sender};
use failure::Error as FailureError;
use faketime::unix_time_as_millis;
Expand Down Expand Up @@ -407,10 +408,14 @@ impl<CS: ChainStore + 'static> BlockAssembler<CS> {
.consensus()
.genesis_epoch_ext()
.block_reward(candidate_number)?,
Bytes::default(),
self.config.data.clone().into_bytes(),
self.shared.consensus().bootstrap_lock().clone(),
None,
);
// User-defined data has a risk of exceeding capacity
if output.is_lack_of_capacity()? {
return Err(TransactionError::InsufficientCellCapacity.into());
}
Ok(TransactionBuilder::default()
.input(input)
.output(output)
Expand All @@ -424,7 +429,16 @@ impl<CS: ChainStore + 'static> BlockAssembler<CS> {

let witness = lock.into_witness();
let input = CellInput::new_cellbase_input(candidate_number);
let output = CellOutput::new(block_reward, Bytes::default(), target_lock, None);
let output = CellOutput::new(
block_reward,
self.config.data.clone().into_bytes(),
target_lock,
None,
);
// User-defined data has a risk of exceeding capacity
if output.is_lack_of_capacity()? {
return Err(TransactionError::InsufficientCellCapacity.into());
}
Ok(TransactionBuilder::default()
.input(input)
.output(output)
Expand Down Expand Up @@ -528,6 +542,7 @@ mod tests {
let config = BlockAssemblerConfig {
code_hash: H256::zero(),
args: vec![],
data: JsonBytes::default(),
};
let mut block_assembler = setup_block_assembler(shared.clone(), config);
let mut candidate_uncles = LruCache::new(MAX_CANDIDATE_UNCLES);
Expand Down Expand Up @@ -648,6 +663,7 @@ mod tests {
let config = BlockAssemblerConfig {
code_hash: H256::zero(),
args: vec![],
data: JsonBytes::default(),
};
let block_assembler = setup_block_assembler(shared.clone(), config);
let new_uncle_receiver = notify.subscribe_new_uncle("test_prepare_uncles");
Expand Down
2 changes: 2 additions & 0 deletions miner/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,4 +13,6 @@ pub struct MinerConfig {
pub struct BlockAssemblerConfig {
pub code_hash: H256,
pub args: Vec<JsonBytes>,
#[serde(default)]
pub data: JsonBytes,
}
8 changes: 8 additions & 0 deletions resource/ckb.toml
Original file line number Diff line number Diff line change
Expand Up @@ -117,8 +117,16 @@ cell_output_cache_size = 128
#
# ckb cli secp256k1-lock <pubkey>
#
# Also, ckb allows the miner to add any data to the cellbase that he has dug out.
driftluo marked this conversation as resolved.
Show resolved Hide resolved
# The data must be A 0x-prefixed hex string. **Please note** that the data cannot be
# larger than the capacity value of the current cellbase. If it is larger,
Copy link
Contributor

Choose a reason for hiding this comment

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

The normal users don't know the capacity value of the cellbase, this data size may be confused for them.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

  1. First of all, it is not a required field here. If you don’t understand it, you will not fill it out.
  2. The capacity of cellbase is dynamically changed. It is impossible to determine the size of data by a static value.
  3. I considered a lot of processing solutions, and finally decided to write the details in the comment, and verify the value when generating the block template to ensure that the user can find the filling error as soon as possible.

Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to truncate the data when it is too long. And add a warning here to tell users that do not rely on the data here, it may be truncated when the block reward capacity is not enough to hold the data.

Copy link
Contributor

@u2 u2 Jun 13, 2019

Choose a reason for hiding this comment

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

It should never be panic if the data size overflow sometimes, for the user view it's a bug.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The current processing will not panic if overflow RPC will return an error when getting the block template.

Copy link
Collaborator Author

@driftluo driftluo Jun 14, 2019

Choose a reason for hiding this comment

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

I changed the implementation to forced truncate, need to review if the calculation of capacity is correct

# the block template cannot be obtained normally. That is, it is impossible to mine normally.
#
# note: The data field is optional.
#
# Below is an example block assembler configuration section:
#
# [block_assembler]
# code_hash = "0xf1951123466e4479842387a66fabfd6b65fc87fd84ae8e6cd3053edb27fff2fd"
# args = ["ckb cli blake160 <pubkey>"]
# data = "A 0x-prefixed hex string"
3 changes: 2 additions & 1 deletion test/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ use ckb_core::header::{HeaderBuilder, Seal};
use ckb_core::script::Script;
use ckb_core::transaction::{CellInput, CellOutput, OutPoint, Transaction, TransactionBuilder};
use ckb_core::{capacity_bytes, BlockNumber, Bytes, Capacity};
use jsonrpc_types::{BlockTemplate, CellbaseTemplate};
use jsonrpc_types::{BlockTemplate, CellbaseTemplate, JsonBytes};
use log::info;
use numext_fixed_hash::H256;
use rand;
Expand Down Expand Up @@ -355,6 +355,7 @@ impl Node {
ckb_config.block_assembler = Some(BlockAssemblerConfig {
code_hash: self.always_success_code_hash.clone(),
args: Default::default(),
data: JsonBytes::default(),
});
modify_ckb_config(&mut ckb_config);
fs::write(
Expand Down
2 changes: 2 additions & 0 deletions test/src/specs/mining/bootstrap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use ckb_app_config::{BlockAssemblerConfig, CKBAppConfig};
use ckb_chain_spec::ChainSpec;
use ckb_core::block::Block;
use ckb_core::script::Script;
use jsonrpc_types::JsonBytes;
use log::info;
use numext_fixed_hash::{h256, H256};

Expand Down Expand Up @@ -65,6 +66,7 @@ impl Spec for BootstrapCellbase {
config.block_assembler = Some(BlockAssemblerConfig {
code_hash: h256!("0xa2"),
args: vec![],
data: JsonBytes::default(),
driftluo marked this conversation as resolved.
Show resolved Hide resolved
});
})
}
Expand Down