Skip to content

Add tests for uncle timestamps and refactor timestamp type #1711

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 25, 2015
Merged

Add tests for uncle timestamps and refactor timestamp type #1711

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 3 additions & 3 deletions cmd/evm/main.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ type VMEnv struct {

depth int
Gas *big.Int
time uint64
time *big.Int
logs []vm.StructLog
}

Expand All @@ -175,15 +175,15 @@ func NewEnv(state *state.StateDB, transactor common.Address, value *big.Int) *VM
state: state,
transactor: &transactor,
value: value,
time: uint64(time.Now().Unix()),
time: big.NewInt(time.Now().Unix()),
}
}

func (self *VMEnv) State() *state.StateDB { return self.state }
func (self *VMEnv) Origin() common.Address { return *self.transactor }
func (self *VMEnv) BlockNumber() *big.Int { return common.Big0 }
func (self *VMEnv) Coinbase() common.Address { return *self.transactor }
func (self *VMEnv) Time() uint64 { return self.time }
func (self *VMEnv) Time() *big.Int { return self.time }
func (self *VMEnv) Difficulty() *big.Int { return common.Big1 }
func (self *VMEnv) BlockHash() []byte { return make([]byte, 32) }
func (self *VMEnv) Value() *big.Int { return self.value }
Expand Down
20 changes: 13 additions & 7 deletions core/block_processor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func (sm *BlockProcessor) processWithParent(block, parent *types.Block) (logs st
txs := block.Transactions()

// Block validation
if err = ValidateHeader(sm.Pow, header, parent, false); err != nil {
if err = ValidateHeader(sm.Pow, header, parent, false, false); err != nil {
return
}

Expand Down Expand Up @@ -327,7 +327,7 @@ func (sm *BlockProcessor) VerifyUncles(statedb *state.StateDB, block, parent *ty
return UncleError("uncle[%d](%x)'s parent is not ancestor (%x)", i, hash[:4], uncle.ParentHash[0:4])
}

if err := ValidateHeader(sm.Pow, uncle, ancestors[uncle.ParentHash], true); err != nil {
if err := ValidateHeader(sm.Pow, uncle, ancestors[uncle.ParentHash], true, true); err != nil {
return ValidationError(fmt.Sprintf("uncle[%d](%x) header invalid: %v", i, hash[:4], err))
}
}
Expand Down Expand Up @@ -358,19 +358,25 @@ func (sm *BlockProcessor) GetLogs(block *types.Block) (logs state.Logs, err erro

// See YP section 4.3.4. "Block Header Validity"
// Validates a block. Returns an error if the block is invalid.
func ValidateHeader(pow pow.PoW, block *types.Header, parent *types.Block, checkPow bool) error {
func ValidateHeader(pow pow.PoW, block *types.Header, parent *types.Block, checkPow, uncle bool) error {
if big.NewInt(int64(len(block.Extra))).Cmp(params.MaximumExtraDataSize) == 1 {
return fmt.Errorf("Block extra data too long (%d)", len(block.Extra))
}

if block.Time > uint64(time.Now().Unix()) {
return BlockFutureErr
if uncle {
if block.Time.Cmp(common.MaxBig) == 1 {
Copy link
Member

Choose a reason for hiding this comment

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

Although valid, comparing against 1 is a bit odd. Usually big int comparison is done as:

  • < 0 if the first is smaller
  • == 0 if they are equal
  • > 0 if the first one is greater

This scheme makes it a bit more visible as the comparison operator already "symbolizes" the actual result vs. having to interpret -1, or 1. It is used like that throughout the entire standard lib too, so better to stick with the canonical use.

Copy link
Author

Choose a reason for hiding this comment

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

@karalabe I'd wait with this since the comparisons for extra data, difficulty and gas uses the "odd" convention - better we change this everywhere in a follow-up PR so we avoid touching unrelated code in this patch.

Copy link
Member

Choose a reason for hiding this comment

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

Ok with me then, but it would be nice to update them at a certain point. :)

return BlockTSTooBigErr
}
} else {
if block.Time.Cmp(big.NewInt(time.Now().Unix())) == 1 {
return BlockFutureErr
}
}
if block.Time <= parent.Time() {
if block.Time.Cmp(parent.Time()) != 1 {
Copy link
Member

Choose a reason for hiding this comment

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

Use <=0 :)

return BlockEqualTSErr
}

expd := CalcDifficulty(block.Time, parent.Time(), parent.Number(), parent.Difficulty())
expd := CalcDifficulty(block.Time.Uint64(), parent.Time().Uint64(), parent.Number(), parent.Difficulty())
if expd.Cmp(block.Difficulty) != 0 {
return fmt.Errorf("Difficulty check failed for block %v, %v", block.Difficulty, expd)
}
Expand Down
4 changes: 2 additions & 2 deletions core/block_processor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,13 +48,13 @@ func TestNumber(t *testing.T) {
statedb := state.New(chain.Genesis().Root(), chain.chainDb)
header := makeHeader(chain.Genesis(), statedb)
header.Number = big.NewInt(3)
err := ValidateHeader(pow, header, chain.Genesis(), false)
err := ValidateHeader(pow, header, chain.Genesis(), false, false)
if err != BlockNumberErr {
t.Errorf("expected block number error, got %q", err)
}

header = makeHeader(chain.Genesis(), statedb)
err = ValidateHeader(pow, header, chain.Genesis(), false)
err = ValidateHeader(pow, header, chain.Genesis(), false, false)
if err == BlockNumberErr {
t.Errorf("didn't expect block number error")
}
Expand Down
11 changes: 8 additions & 3 deletions core/chain_makers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,16 +166,21 @@ func GenerateChain(parent *types.Block, db common.Database, n int, gen func(int,
}

func makeHeader(parent *types.Block, state *state.StateDB) *types.Header {
time := parent.Time() + 10 // block time is fixed at 10 seconds
var time *big.Int
if parent.Time() == nil {
time = big.NewInt(10)
} else {
time = new(big.Int).Add(parent.Time(), big.NewInt(10)) // block time is fixed at 10 seconds
}
return &types.Header{
Root: state.Root(),
ParentHash: parent.Hash(),
Coinbase: parent.Coinbase(),
Difficulty: CalcDifficulty(time, parent.Time(), parent.Number(), parent.Difficulty()),
Difficulty: CalcDifficulty(time.Uint64(), new(big.Int).Sub(time, big.NewInt(10)).Uint64(), parent.Number(), parent.Difficulty()),
Copy link
Contributor

Choose a reason for hiding this comment

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

2nd arg why not parent.Time().Uint64()?

Copy link
Author

Choose a reason for hiding this comment

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

Some tests do not initialise parent in the first call to this function, and I wanted to avoid changing them just for this patch.

GasLimit: CalcGasLimit(parent),
GasUsed: new(big.Int),
Number: new(big.Int).Add(parent.Number(), common.Big1),
Time: uint64(time),
Time: time,
}
}

Expand Down
3 changes: 2 additions & 1 deletion core/chain_manager.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -596,7 +596,8 @@ func (self *ChainManager) InsertChain(chain types.Blocks) (int, error) {
// Allow up to MaxFuture second in the future blocks. If this limit
// is exceeded the chain is discarded and processed at a later time
// if given.
if max := uint64(time.Now().Unix()) + maxTimeFutureBlocks; block.Time() > max {
max := big.NewInt(time.Now().Unix() + maxTimeFutureBlocks)
if block.Time().Cmp(max) == 1 {
return i, fmt.Errorf("%v: BlockFutureErr, %v > %v", BlockFutureErr, block.Time(), max)
}

Expand Down
7 changes: 4 additions & 3 deletions core/error.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ import (
)

var (
BlockNumberErr = errors.New("block number invalid")
BlockFutureErr = errors.New("block time is in the future")
BlockEqualTSErr = errors.New("block time stamp equal to previous")
BlockNumberErr = errors.New("block number invalid")
BlockFutureErr = errors.New("block time is in the future")
BlockTSTooBigErr = errors.New("block time too big")
BlockEqualTSErr = errors.New("block time stamp equal to previous")
)

// Parent error. In case a parent is unknown this error will be thrown
Expand Down
2 changes: 1 addition & 1 deletion core/genesis.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ func WriteGenesisBlock(chainDb common.Database, reader io.Reader) (*types.Block,
difficulty := common.String2Big(genesis.Difficulty)
block := types.NewBlock(&types.Header{
Nonce: types.EncodeNonce(common.String2Big(genesis.Nonce).Uint64()),
Time: common.String2Big(genesis.Timestamp).Uint64(),
Time: common.String2Big(genesis.Timestamp),
ParentHash: common.HexToHash(genesis.ParentHash),
Extra: common.FromHex(genesis.ExtraData),
GasLimit: common.String2Big(genesis.GasLimit),
Expand Down
9 changes: 6 additions & 3 deletions core/types/block.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ type Header struct {
Number *big.Int // The block number
GasLimit *big.Int // Gas limit
GasUsed *big.Int // Gas used
Time uint64 // Creation time
Time *big.Int // Creation time
Extra []byte // Extra data
MixDigest common.Hash // for quick difficulty verification
Nonce BlockNonce
Expand Down Expand Up @@ -94,7 +94,7 @@ func (h *Header) UnmarshalJSON(data []byte) error {
Coinbase string
Difficulty string
GasLimit string
Time uint64
Time *big.Int
Extra string
}
dec := json.NewDecoder(bytes.NewReader(data))
Expand Down Expand Up @@ -210,6 +210,9 @@ func NewBlockWithHeader(header *Header) *Block {

func copyHeader(h *Header) *Header {
cpy := *h
if cpy.Time = new(big.Int); h.Time != nil {
cpy.Time.Set(h.Time)
}
if cpy.Difficulty = new(big.Int); h.Difficulty != nil {
cpy.Difficulty.Set(h.Difficulty)
}
Expand Down Expand Up @@ -301,13 +304,13 @@ func (b *Block) Number() *big.Int { return new(big.Int).Set(b.header.Number)
func (b *Block) GasLimit() *big.Int { return new(big.Int).Set(b.header.GasLimit) }
func (b *Block) GasUsed() *big.Int { return new(big.Int).Set(b.header.GasUsed) }
func (b *Block) Difficulty() *big.Int { return new(big.Int).Set(b.header.Difficulty) }
func (b *Block) Time() *big.Int { return new(big.Int).Set(b.header.Time) }
Copy link
Contributor

Choose a reason for hiding this comment

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

@Gustav-Simonsson please also update copyHeader

Copy link
Author

Choose a reason for hiding this comment

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

done


func (b *Block) NumberU64() uint64 { return b.header.Number.Uint64() }
func (b *Block) MixDigest() common.Hash { return b.header.MixDigest }
func (b *Block) Nonce() uint64 { return binary.BigEndian.Uint64(b.header.Nonce[:]) }
func (b *Block) Bloom() Bloom { return b.header.Bloom }
func (b *Block) Coinbase() common.Address { return b.header.Coinbase }
func (b *Block) Time() uint64 { return b.header.Time }
func (b *Block) Root() common.Hash { return b.header.Root }
func (b *Block) ParentHash() common.Hash { return b.header.ParentHash }
func (b *Block) TxHash() common.Hash { return b.header.TxHash }
Expand Down
2 changes: 1 addition & 1 deletion core/types/block_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ func TestBlockEncoding(t *testing.T) {
check("Root", block.Root(), common.HexToHash("ef1552a40b7165c3cd773806b9e0c165b75356e0314bf0706f279c729f51e017"))
check("Hash", block.Hash(), common.HexToHash("0a5843ac1cb04865017cb35a57b50b07084e5fcee39b5acadade33149f4fff9e"))
check("Nonce", block.Nonce(), uint64(0xa13a5a8c8f2bb1c4))
check("Time", block.Time(), uint64(1426516743))
check("Time", block.Time(), big.NewInt(1426516743))
check("Size", block.Size(), common.StorageSize(len(blockEnc)))

tx1 := NewTransaction(0, common.HexToAddress("095e7baea6a6c7c4c2dfeb977efac326af552d87"), big.NewInt(10), big.NewInt(50000), big.NewInt(10), nil)
Expand Down
2 changes: 1 addition & 1 deletion core/vm/environment.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ type Environment interface {
BlockNumber() *big.Int
GetHash(n uint64) common.Hash
Coinbase() common.Address
Time() uint64
Time() *big.Int
Difficulty() *big.Int
GasLimit() *big.Int
CanTransfer(from Account, balance *big.Int) bool
Expand Down
2 changes: 1 addition & 1 deletion core/vm/instructions.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,7 +341,7 @@ func opCoinbase(instr instruction, env Environment, context *Context, memory *Me
}

func opTimestamp(instr instruction, env Environment, context *Context, memory *Memory, stack *stack) {
stack.push(U256(new(big.Int).SetUint64(env.Time())))
stack.push(U256(new(big.Int).Set(env.Time())))
}

func opNumber(instr instruction, env Environment, context *Context, memory *Memory, stack *stack) {
Expand Down
2 changes: 1 addition & 1 deletion core/vm/jit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ func (self *Env) StructLogs() []StructLog {

//func (self *Env) PrevHash() []byte { return self.parent }
func (self *Env) Coinbase() common.Address { return common.Address{} }
func (self *Env) Time() uint64 { return uint64(time.Now().Unix()) }
func (self *Env) Time() *big.Int { return big.NewInt(time.Now().Unix()) }
func (self *Env) Difficulty() *big.Int { return big.NewInt(0) }
func (self *Env) State() *state.StateDB { return nil }
func (self *Env) GasLimit() *big.Int { return self.gasLimit }
Expand Down
2 changes: 1 addition & 1 deletion core/vm/vm.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,7 @@ func (self *Vm) Run(context *Context, input []byte) (ret []byte, err error) {
case TIMESTAMP:
time := self.env.Time()

stack.push(new(big.Int).SetUint64(time))
stack.push(new(big.Int).Set(time))

case NUMBER:
number := self.env.BlockNumber()
Expand Down
2 changes: 1 addition & 1 deletion core/vm_env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ func NewEnv(state *state.StateDB, chain *ChainManager, msg Message, header *type
func (self *VMEnv) Origin() common.Address { f, _ := self.msg.From(); return f }
func (self *VMEnv) BlockNumber() *big.Int { return self.header.Number }
func (self *VMEnv) Coinbase() common.Address { return self.header.Coinbase }
func (self *VMEnv) Time() uint64 { return self.header.Time }
func (self *VMEnv) Time() *big.Int { return self.header.Time }
Copy link
Member

Choose a reason for hiding this comment

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

@fjl Here nonce of the big ints are immutable (i.e. copied). Shouldn't we update these too while at it? Or maybe in a follow up PR?

Copy link
Author

Choose a reason for hiding this comment

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

@karalabe nonce? The VMEnv is meant to be mutable afaik.

func (self *VMEnv) Difficulty() *big.Int { return self.header.Difficulty }
func (self *VMEnv) GasLimit() *big.Int { return self.header.GasLimit }
func (self *VMEnv) Value() *big.Int { return self.msg.Value() }
Expand Down
2 changes: 1 addition & 1 deletion eth/handler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,7 @@ func NewProtocolManager(networkId int, mux *event.TypeMux, txpool txPool, pow po
manager.downloader = downloader.New(manager.eventMux, manager.chainman.HasBlock, manager.chainman.GetBlock, manager.chainman.CurrentBlock, manager.chainman.InsertChain, manager.removePeer)

validator := func(block *types.Block, parent *types.Block) error {
return core.ValidateHeader(pow, block.Header(), parent, true)
return core.ValidateHeader(pow, block.Header(), parent, true, false)
}
heighter := func() uint64 {
return manager.chainman.CurrentBlock().NumberU64()
Expand Down
10 changes: 5 additions & 5 deletions miner/worker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ func (self *worker) wait() {
glog.V(logger.Error).Infoln("Invalid block found during mining")
continue
}
if err := core.ValidateHeader(self.eth.BlockProcessor().Pow, block.Header(), parent, true); err != nil && err != core.BlockFutureErr {
if err := core.ValidateHeader(self.eth.BlockProcessor().Pow, block.Header(), parent, true, false); err != nil && err != core.BlockFutureErr {
glog.V(logger.Error).Infoln("Invalid header on mined block:", err)
continue
}
Expand Down Expand Up @@ -434,8 +434,8 @@ func (self *worker) commitNewWork() {
tstart := time.Now()
parent := self.chain.CurrentBlock()
tstamp := tstart.Unix()
if tstamp <= int64(parent.Time()) {
tstamp = int64(parent.Time()) + 1
if parent.Time().Cmp(new(big.Int).SetInt64(tstamp)) != 1 {
tstamp = parent.Time().Int64() + 1
}
// this will ensure we're not going off too far in the future
if now := time.Now().Unix(); tstamp > now+4 {
Expand All @@ -448,12 +448,12 @@ func (self *worker) commitNewWork() {
header := &types.Header{
ParentHash: parent.Hash(),
Number: num.Add(num, common.Big1),
Difficulty: core.CalcDifficulty(uint64(tstamp), parent.Time(), parent.Number(), parent.Difficulty()),
Difficulty: core.CalcDifficulty(uint64(tstamp), parent.Time().Uint64(), parent.Number(), parent.Difficulty()),
GasLimit: core.CalcGasLimit(parent),
GasUsed: new(big.Int),
Coinbase: self.coinbase,
Extra: self.extra,
Time: uint64(tstamp),
Time: big.NewInt(tstamp),
}

previous := self.current
Expand Down
7 changes: 7 additions & 0 deletions tests/block_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ func TestBcUncleHeaderValidityTests(t *testing.T) {
}
}

func TestBcUncleTests(t *testing.T) {
err := RunBlockTest(filepath.Join(blockTestDir, "bcUncleTest.json"), BlockSkipTests)
if err != nil {
t.Fatal(err)
}
}

func TestBcInvalidHeaderTests(t *testing.T) {
err := RunBlockTest(filepath.Join(blockTestDir, "bcInvalidHeaderTest.json"), BlockSkipTests)
if err != nil {
Expand Down
6 changes: 3 additions & 3 deletions tests/block_test_util.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -365,8 +365,8 @@ func (s *BlockTest) validateBlockHeader(h *btHeader, h2 *types.Header) error {
return fmt.Errorf("GasUsed: expected: %v, decoded: %v", expectedGasUsed, h2.GasUsed)
}

expectedTimestamp := mustConvertUint(h.Timestamp, 16)
if expectedTimestamp != h2.Time {
expectedTimestamp := mustConvertBigInt(h.Timestamp, 16)
if expectedTimestamp.Cmp(h2.Time) != 0 {
return fmt.Errorf("Timestamp: expected: %v, decoded: %v", expectedTimestamp, h2.Time)
}

Expand Down Expand Up @@ -461,7 +461,7 @@ func mustConvertHeader(in btHeader) *types.Header {
GasUsed: mustConvertBigInt(in.GasUsed, 16),
GasLimit: mustConvertBigInt(in.GasLimit, 16),
Difficulty: mustConvertBigInt(in.Difficulty, 16),
Time: mustConvertUint(in.Timestamp, 16),
Time: mustConvertBigInt(in.Timestamp, 16),
Nonce: types.EncodeNonce(mustConvertUint(in.Nonce, 16)),
}
return header
Expand Down
Loading