From 02ba0b309ce666c62bb47cbba5500fcbcb5fae94 Mon Sep 17 00:00:00 2001 From: Eugene Kim Date: Thu, 5 Dec 2019 11:07:02 +0000 Subject: [PATCH 1/5] Fix cross-TX header introduction glitch Commit e79ba5fe880b56973a50a80d918b9145fce07cf0 introduced an off-by-one error which caused CX-enabled header to be introduced one epoch later than expected, i.e. at CrossTxEpoch + 1, where it should be CrossTxEpoch. This was caused by a vague, confusing naming of IsCrossTx(epoch) function of ChainConfig: The name did not clarify whether it meant introduction of CX-enabled header and processing of ingress receipts (which occurs at CrossTxEpoch) or acceptance of cross-shard transactions (which occurs at CrossTxEpoch+1). Introduce and use params.(*ChainConfig).HasCrossTxFields which handles the former case. --- block/factory/factory.go | 2 +- internal/params/config.go | 17 ++++++++++++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/block/factory/factory.go b/block/factory/factory.go index 6c3e3aac9..8de3fad0a 100644 --- a/block/factory/factory.go +++ b/block/factory/factory.go @@ -34,7 +34,7 @@ func (f *factory) NewHeader(epoch *big.Int) *block.Header { impl = v3.NewHeader() case f.chainConfig.IsCrossLink(epoch): impl = v2.NewHeader() - case f.chainConfig.IsCrossTx(epoch): + case f.chainConfig.HasCrossTxFields(epoch): impl = v1.NewHeader() default: impl = v0.NewHeader() diff --git a/internal/params/config.go b/internal/params/config.go index 5e5926426..99d6e23ec 100644 --- a/internal/params/config.go +++ b/internal/params/config.go @@ -139,21 +139,28 @@ func (c *ChainConfig) IsEIP155(epoch *big.Int) bool { return isForked(c.EIP155Epoch, epoch) } -// IsCrossTx returns whether cross-shard transaction is enabled in the given +// IsCrossTx returns whether cross-shard transaction is accepted in the given // epoch. // // Note that this is different from comparing epoch against CrossTxEpoch. -// Cross-shard transaction is enabled from CrossTxEpoch+1 and on, in order to +// Cross-shard transaction is accepted from CrossTxEpoch+1 and on, in order to // allow for all shards to roll into CrossTxEpoch and become able to handle // ingress receipts. In other words, cross-shard transaction fields are -// introduced at CrossTxEpoch, but these fields are not used until -// CrossTxEpoch+1, when cross-shard transactions are actually accepted by the -// network. +// introduced and ingress receipts are processed at CrossTxEpoch, but the shard +// does not accept cross-shard transactions from clients until CrossTxEpoch+1. +// +// TODO ek – rename to AcceptsCrossTx to clarify func (c *ChainConfig) IsCrossTx(epoch *big.Int) bool { crossTxEpoch := new(big.Int).Add(c.CrossTxEpoch, common.Big1) return isForked(crossTxEpoch, epoch) } +// HasCrossTxFields returns whether blocks in the given epoch includes +// cross-shard transaction fields. +func (c *ChainConfig) HasCrossTxFields(epoch *big.Int) bool { + return isForked(c.CrossTxEpoch, epoch) +} + // IsStaking determines whether it is staking epoch func (c *ChainConfig) IsStaking(epoch *big.Int) bool { return isForked(c.StakingEpoch, epoch) From ef389aa9b54a2121324c98a45a7a060e3516582b Mon Sep 17 00:00:00 2001 From: Eugene Kim Date: Thu, 5 Dec 2019 03:18:24 -0800 Subject: [PATCH 2/5] =?UTF-8?q?IsCrossTx=20=E2=86=92=20AcceptsCrossTx?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This clarifies the intended use of the predicate. --- core/block_validator.go | 4 ++-- core/blockchain.go | 2 +- core/state_processor.go | 4 ++-- internal/params/config.go | 8 +++----- node/node_newblock.go | 2 +- 5 files changed, 9 insertions(+), 11 deletions(-) diff --git a/core/block_validator.go b/core/block_validator.go index d6009b986..9baf709a3 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -100,7 +100,7 @@ func (v *BlockValidator) ValidateState(block, parent *types.Block, statedb *stat return fmt.Errorf("invalid receipt root hash (remote: %x local: %x)", header.ReceiptHash(), receiptSha) } - if v.config.IsCrossTx(block.Epoch()) { + if v.config.AcceptsCrossTx(block.Epoch()) { cxsSha := types.DeriveMultipleShardsSha(cxReceipts) if cxsSha != header.OutgoingReceiptHash() { return fmt.Errorf("invalid cross shard receipt root hash (remote: %x local: %x)", header.OutgoingReceiptHash(), cxsSha) @@ -175,7 +175,7 @@ func CalcGasLimit(parent *types.Block, gasFloor, gasCeil uint64) uint64 { // ValidateCXReceiptsProof checks whether the given CXReceiptsProof is consistency with itself func (v *BlockValidator) ValidateCXReceiptsProof(cxp *types.CXReceiptsProof) error { - if !v.config.IsCrossTx(cxp.Header.Epoch()) { + if !v.config.AcceptsCrossTx(cxp.Header.Epoch()) { return ctxerror.New("[ValidateCXReceiptsProof] cross shard receipt received before cx fork") } diff --git a/core/blockchain.go b/core/blockchain.go index 02bd4cb0b..29a09531a 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1102,7 +1102,7 @@ func (bc *BlockChain) WriteBlockWithState( //// Cross-shard txns epoch := block.Header().Epoch() - if bc.chainConfig.IsCrossTx(block.Epoch()) { + if bc.chainConfig.AcceptsCrossTx(block.Epoch()) { shardingConfig := shard.Schedule.InstanceForEpoch(epoch) shardNum := int(shardingConfig.NumShards()) for i := 0; i < shardNum; i++ { diff --git a/core/state_processor.go b/core/state_processor.go index 713a8d511..f7545fb4e 100644 --- a/core/state_processor.go +++ b/core/state_processor.go @@ -122,7 +122,7 @@ func (p *StateProcessor) Process(block *types.Block, statedb *state.DB, cfg vm.C // return true if it is valid func getTransactionType(config *params.ChainConfig, header *block.Header, tx *types.Transaction) types.TransactionType { - if header.ShardID() == tx.ShardID() && (!config.IsCrossTx(header.Epoch()) || tx.ShardID() == tx.ToShardID()) { + if header.ShardID() == tx.ShardID() && (!config.AcceptsCrossTx(header.Epoch()) || tx.ShardID() == tx.ToShardID()) { return types.SameShardTx } numShards := shard.Schedule.InstanceForEpoch(header.Epoch()).NumShards() @@ -143,7 +143,7 @@ func ApplyTransaction(config *params.ChainConfig, bc ChainContext, author *commo return nil, nil, 0, fmt.Errorf("Invalid Transaction Type") } - if txType != types.SameShardTx && !config.IsCrossTx(header.Epoch()) { + if txType != types.SameShardTx && !config.AcceptsCrossTx(header.Epoch()) { return nil, nil, 0, fmt.Errorf( "cannot handle cross-shard transaction until after epoch %v (now %v)", config.CrossTxEpoch, header.Epoch()) diff --git a/internal/params/config.go b/internal/params/config.go index 99d6e23ec..8c7e8ef5c 100644 --- a/internal/params/config.go +++ b/internal/params/config.go @@ -139,8 +139,8 @@ func (c *ChainConfig) IsEIP155(epoch *big.Int) bool { return isForked(c.EIP155Epoch, epoch) } -// IsCrossTx returns whether cross-shard transaction is accepted in the given -// epoch. +// AcceptsCrossTx returns whether cross-shard transaction is accepted in the +// given epoch. // // Note that this is different from comparing epoch against CrossTxEpoch. // Cross-shard transaction is accepted from CrossTxEpoch+1 and on, in order to @@ -148,9 +148,7 @@ func (c *ChainConfig) IsEIP155(epoch *big.Int) bool { // ingress receipts. In other words, cross-shard transaction fields are // introduced and ingress receipts are processed at CrossTxEpoch, but the shard // does not accept cross-shard transactions from clients until CrossTxEpoch+1. -// -// TODO ek – rename to AcceptsCrossTx to clarify -func (c *ChainConfig) IsCrossTx(epoch *big.Int) bool { +func (c *ChainConfig) AcceptsCrossTx(epoch *big.Int) bool { crossTxEpoch := new(big.Int).Add(c.CrossTxEpoch, common.Big1) return isForked(crossTxEpoch, epoch) } diff --git a/node/node_newblock.go b/node/node_newblock.go index bcedeab6f..a036ea55b 100644 --- a/node/node_newblock.go +++ b/node/node_newblock.go @@ -167,7 +167,7 @@ func (node *Node) proposeNewBlock() (*types.Block, error) { } func (node *Node) proposeReceiptsProof() []*types.CXReceiptsProof { - if !node.Blockchain().Config().IsCrossTx(node.Worker.GetCurrentHeader().Epoch()) { + if !node.Blockchain().Config().AcceptsCrossTx(node.Worker.GetCurrentHeader().Epoch()) { return []*types.CXReceiptsProof{} } From 43c9bb46fce5273c2973b817baf7e8619fae5a62 Mon Sep 17 00:00:00 2001 From: Eugene Kim Date: Thu, 5 Dec 2019 03:25:06 -0800 Subject: [PATCH 3/5] Validate CX receipt root from CrossTxEpoch Previously, validation started at CrossTxEpoch+1 and CX receipts roots were unverified in CrossTxEpoch, creating a security hole. --- core/block_validator.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/block_validator.go b/core/block_validator.go index 9baf709a3..afb1ab3be 100644 --- a/core/block_validator.go +++ b/core/block_validator.go @@ -100,7 +100,7 @@ func (v *BlockValidator) ValidateState(block, parent *types.Block, statedb *stat return fmt.Errorf("invalid receipt root hash (remote: %x local: %x)", header.ReceiptHash(), receiptSha) } - if v.config.AcceptsCrossTx(block.Epoch()) { + if v.config.HasCrossTxFields(block.Epoch()) { cxsSha := types.DeriveMultipleShardsSha(cxReceipts) if cxsSha != header.OutgoingReceiptHash() { return fmt.Errorf("invalid cross shard receipt root hash (remote: %x local: %x)", header.OutgoingReceiptHash(), cxsSha) From 4a326b33e49419803d80000f849f4cb31a255fc7 Mon Sep 17 00:00:00 2001 From: Eugene Kim Date: Thu, 5 Dec 2019 03:27:52 -0800 Subject: [PATCH 4/5] Store CX receipts in blocks in CrossTxEpoch Previously, CX receipt storage started at CrossTxEpoch+1, causing incoming CX receipts in blocks in CrossTxEpoch to be lost. --- core/blockchain.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/blockchain.go b/core/blockchain.go index 29a09531a..40fb0a283 100644 --- a/core/blockchain.go +++ b/core/blockchain.go @@ -1102,7 +1102,7 @@ func (bc *BlockChain) WriteBlockWithState( //// Cross-shard txns epoch := block.Header().Epoch() - if bc.chainConfig.AcceptsCrossTx(block.Epoch()) { + if bc.chainConfig.HasCrossTxFields(block.Epoch()) { shardingConfig := shard.Schedule.InstanceForEpoch(epoch) shardNum := int(shardingConfig.NumShards()) for i := 0; i < shardNum; i++ { From 77354c48aa373ed378a5892c2bb21d10e8a2950f Mon Sep 17 00:00:00 2001 From: Eugene Kim Date: Thu, 5 Dec 2019 03:30:22 -0800 Subject: [PATCH 5/5] Start processing ingress CXRs at CrossTxEpoch Previously, ingress CX receipts began to be processed in CrossTxEpoch+1, causing processing of early-day CXs to be delayed up to a full epoch. --- node/node_newblock.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/node_newblock.go b/node/node_newblock.go index a036ea55b..56ba0b352 100644 --- a/node/node_newblock.go +++ b/node/node_newblock.go @@ -167,7 +167,7 @@ func (node *Node) proposeNewBlock() (*types.Block, error) { } func (node *Node) proposeReceiptsProof() []*types.CXReceiptsProof { - if !node.Blockchain().Config().AcceptsCrossTx(node.Worker.GetCurrentHeader().Epoch()) { + if !node.Blockchain().Config().HasCrossTxFields(node.Worker.GetCurrentHeader().Epoch()) { return []*types.CXReceiptsProof{} }