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

core: add status as a consensus field in receipt #15014

Merged
merged 3 commits into from
Aug 23, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
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
3 changes: 2 additions & 1 deletion accounts/abi/bind/backends/simulated.go
Original file line number Diff line number Diff line change
Expand Up @@ -253,7 +253,8 @@ func (b *SimulatedBackend) callContract(ctx context.Context, call ethereum.CallM
// about the transaction and calling mechanisms.
vmenv := vm.NewEVM(evmContext, statedb, b.config, vm.Config{})
gaspool := new(core.GasPool).AddGas(math.MaxBig256)
ret, gasUsed, _, err := core.NewStateTransition(vmenv, msg, gaspool).TransitionDb()
// TODO utilize returned failed flag to help gas estimation.
ret, gasUsed, _, _, err := core.NewStateTransition(vmenv, msg, gaspool).TransitionDb()
return ret, gasUsed, err
}

Expand Down
3 changes: 3 additions & 0 deletions common/bytes.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,9 @@ func FromHex(s string) []byte {
//
// Returns an exact copy of the provided bytes
func CopyBytes(b []byte) (copiedBytes []byte) {
if b == nil {
return nil
}
copiedBytes = make([]byte, len(b))
copy(copiedBytes, b)

Expand Down
4 changes: 2 additions & 2 deletions core/database_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -461,12 +461,12 @@ func TestMipmapChain(t *testing.T) {
var receipts types.Receipts
switch i {
case 1:
receipt := types.NewReceipt(nil, new(big.Int))
receipt := types.NewReceipt(nil, false, new(big.Int))
receipt.Logs = []*types.Log{{Address: addr, Topics: []common.Hash{hash1}}}
gen.AddUncheckedReceipt(receipt)
receipts = types.Receipts{receipt}
case 1000:
receipt := types.NewReceipt(nil, new(big.Int))
receipt := types.NewReceipt(nil, false, new(big.Int))
receipt.Logs = []*types.Log{{Address: addr2}}
gen.AddUncheckedReceipt(receipt)
receipts = types.Receipts{receipt}
Expand Down
4 changes: 2 additions & 2 deletions core/state_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ func ApplyTransaction(config *params.ChainConfig, bc *BlockChain, author *common
// about the transaction and calling mechanisms.
vmenv := vm.NewEVM(context, statedb, config, cfg)
// Apply the transaction to the current state (included in the env)
_, gas, err := ApplyMessage(vmenv, msg, gp)
_, gas, failed, err := ApplyMessage(vmenv, msg, gp)
if err != nil {
return nil, nil, err
}
Expand All @@ -114,7 +114,7 @@ func ApplyTransaction(config *params.ChainConfig, bc *BlockChain, author *common

// Create a new receipt for the transaction, storing the intermediate root and gas used by the tx
// based on the eip phase, we're passing wether the root touch-delete accounts.
receipt := types.NewReceipt(root, usedGas)
receipt := types.NewReceipt(root, failed, usedGas)
receipt.TxHash = tx.Hash()
receipt.GasUsed = new(big.Int).Set(gas)
// if the transaction created a contract, store the creation address in the receipt.
Expand Down
19 changes: 9 additions & 10 deletions core/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,8 +59,7 @@ type StateTransition struct {
value *big.Int
data []byte
state vm.StateDB

evm *vm.EVM
evm *vm.EVM
}

// Message represents a message sent to a contract.
Expand Down Expand Up @@ -127,11 +126,11 @@ func NewStateTransition(evm *vm.EVM, msg Message, gp *GasPool) *StateTransition
// the gas used (which includes gas refunds) and an error if it failed. An error always
// indicates a core error meaning that the message would always fail for that particular
// state and would never be accepted within a block.
func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool) ([]byte, *big.Int, error) {
func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool) ([]byte, *big.Int, bool, error) {
st := NewStateTransition(evm, msg, gp)

ret, _, gasUsed, err := st.TransitionDb()
return ret, gasUsed, err
ret, _, gasUsed, failed, err := st.TransitionDb()
return ret, gasUsed, failed, err
}

func (st *StateTransition) from() vm.AccountRef {
Expand Down Expand Up @@ -208,7 +207,7 @@ func (st *StateTransition) preCheck() error {
// TransitionDb will transition the state by applying the current message and returning the result
// including the required gas for the operation as well as the used gas. It returns an error if it
// failed. An error indicates a consensus issue.
func (st *StateTransition) TransitionDb() (ret []byte, requiredGas, usedGas *big.Int, err error) {
func (st *StateTransition) TransitionDb() (ret []byte, requiredGas, usedGas *big.Int, failed bool, err error) {
if err = st.preCheck(); err != nil {
return
}
Expand All @@ -222,10 +221,10 @@ func (st *StateTransition) TransitionDb() (ret []byte, requiredGas, usedGas *big
// TODO convert to uint64
intrinsicGas := IntrinsicGas(st.data, contractCreation, homestead)
if intrinsicGas.BitLen() > 64 {
return nil, nil, nil, vm.ErrOutOfGas
return nil, nil, nil, false, vm.ErrOutOfGas
}
if err = st.useGas(intrinsicGas.Uint64()); err != nil {
return nil, nil, nil, err
return nil, nil, nil, false, err
}

var (
Expand All @@ -248,15 +247,15 @@ func (st *StateTransition) TransitionDb() (ret []byte, requiredGas, usedGas *big
// sufficient balance to make the transfer happen. The first
// balance transfer may never fail.
if vmerr == vm.ErrInsufficientBalance {
return nil, nil, nil, vmerr
return nil, nil, nil, false, vmerr
}
}
requiredGas = new(big.Int).Set(st.gasUsed())

st.refundGas()
st.state.AddBalance(st.evm.Coinbase, new(big.Int).Mul(st.gasUsed(), st.gasPrice))

return ret, requiredGas, st.gasUsed(), err
return ret, requiredGas, st.gasUsed(), vmerr != nil, err
}

func (st *StateTransition) refundGas() {
Expand Down
6 changes: 6 additions & 0 deletions core/types/gen_receipt_json.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

44 changes: 29 additions & 15 deletions core/types/receipt.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,16 @@ import (

//go:generate gencodec -type Receipt -field-override receiptMarshaling -out gen_receipt_json.go

const (
receiptStatusSuccessful = byte(0x01)
receiptStatusFailed = byte(0x00)
)
Copy link
Member

Choose a reason for hiding this comment

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

These should be private (since we want to work with bools API wise) and please document them.


// Receipt represents the results of a transaction.
type Receipt struct {
// Consensus fields
PostState []byte `json:"root"`
Failed bool `json:"failed"`
CumulativeGasUsed *big.Int `json:"cumulativeGasUsed" gencodec:"required"`
Bloom Bloom `json:"logsBloom" gencodec:"required"`
Logs []*Log `json:"logs" gencodec:"required"`
Expand Down Expand Up @@ -60,21 +66,26 @@ type homesteadReceiptRLP struct {
// metropolisReceiptRLP contains the receipt's Metropolis consensus fields, used
// during RLP serialization.
type metropolisReceiptRLP struct {
Status byte
CumulativeGasUsed *big.Int
Bloom Bloom
Logs []*Log
}

// NewReceipt creates a barebone transaction receipt, copying the init fields.
func NewReceipt(root []byte, cumulativeGasUsed *big.Int) *Receipt {
return &Receipt{PostState: common.CopyBytes(root), CumulativeGasUsed: new(big.Int).Set(cumulativeGasUsed)}
func NewReceipt(root []byte, failed bool, cumulativeGasUsed *big.Int) *Receipt {
return &Receipt{PostState: common.CopyBytes(root), Failed: failed, CumulativeGasUsed: new(big.Int).Set(cumulativeGasUsed)}
}

// EncodeRLP implements rlp.Encoder, and flattens the consensus fields of a receipt
// into an RLP stream. If no post state is present, metropolis fork is assumed.
func (r *Receipt) EncodeRLP(w io.Writer) error {
if r.PostState == nil {
return rlp.Encode(w, &metropolisReceiptRLP{r.CumulativeGasUsed, r.Bloom, r.Logs})
status := receiptStatusSuccessful
if r.Failed {
status = receiptStatusFailed
}
Copy link
Member

Choose a reason for hiding this comment

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

This can be done a bit more compactly with:

status := receiptStatusSuccessful
if r.Failed {
  status = receiptStatusFailed
}

return rlp.Encode(w, &metropolisReceiptRLP{status, r.CumulativeGasUsed, r.Bloom, r.Logs})
}
return rlp.Encode(w, &homesteadReceiptRLP{r.PostState, r.CumulativeGasUsed, r.Bloom, r.Logs})
}
Expand All @@ -87,29 +98,31 @@ func (r *Receipt) DecodeRLP(s *rlp.Stream) error {
if err != nil {
return err
}
list, _, err := rlp.SplitList(raw)
content, _, err := rlp.SplitList(raw)
if err != nil {
return err
}
items, err := rlp.CountValues(list)
kind, cnt, _, err := rlp.Split(content)
if err != nil {
return err
}
// Deserialize based on the number of content items
switch items {
case 3:
// Metropolis receipts have 3 components
// Deserialize based on the first component type.
switch {
case kind == rlp.Byte || kind == rlp.String && len(cnt) == 0:
Copy link
Member

Choose a reason for hiding this comment

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

You don't need the string magic here, it's only byte we're working with.

Copy link
Contributor

Choose a reason for hiding this comment

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

This should definitely be removed, or else we'll have a potential consensus failure by accepting an invalid receipt.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Arachnid Since now the empty value is encoded as 0x80. When we use rlp.Split, it returns a String type instead of the byte type we expected. So add these statements in order to deal with this situation.

Copy link
Member Author

Choose a reason for hiding this comment

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

@Arachnid This is another solution to fix it. Change receipt status type from byte to []byte. Since rlp will treat byte as uint8, and byte(0x00) will be encoded as 0x80, which make confused. If we use []byte to represent receipt status, []byte{0x00} can be encoded as 0x00 correctly. But the receipt definition can cause ambiguity.

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 this part is correct, since 0x01 is decoded as a byte, but 0x80 as an empty string (0 number). It's a bit weird, but that's the way RLP guesses the type unless it's given explicitly.

One thing that did occur to me however, is that we don't enforce the status byte to be 0 or 1. That's a possible consensus problem. I'll add a fix for this.

// The first component of metropolis receipts is Byte
// or empty String(byte with 0x00 value).
var metro metropolisReceiptRLP
if err := rlp.DecodeBytes(raw, &metro); err != nil {
return err
}
r.Failed = metro.Status == receiptStatusFailed
r.CumulativeGasUsed = metro.CumulativeGasUsed
r.Bloom = metro.Bloom
r.Logs = metro.Logs
return nil

case 4:
// Homestead receipts have 4 components
case kind == rlp.String:
// The first component of homestead receipts is non-empty String.
var home homesteadReceiptRLP
if err := rlp.DecodeBytes(raw, &home); err != nil {
return err
Expand All @@ -121,14 +134,14 @@ func (r *Receipt) DecodeRLP(s *rlp.Stream) error {
return nil

default:
return fmt.Errorf("invalid receipt components: %v", items)
return fmt.Errorf("invalid first receipt component: %v", kind)
}
}

// String implements the Stringer interface.
func (r *Receipt) String() string {
if r.PostState == nil {
return fmt.Sprintf("receipt{cgas=%v bloom=%x logs=%v}", r.CumulativeGasUsed, r.Bloom, r.Logs)
return fmt.Sprintf("receipt{failed=%t cgas=%v bloom=%x logs=%v}", r.Failed, r.CumulativeGasUsed, r.Bloom, r.Logs)
}
return fmt.Sprintf("receipt{med=%x cgas=%v bloom=%x logs=%v}", r.PostState, r.CumulativeGasUsed, r.Bloom, r.Logs)
}
Expand All @@ -144,14 +157,15 @@ func (r *ReceiptForStorage) EncodeRLP(w io.Writer) error {
for i, log := range r.Logs {
logs[i] = (*LogForStorage)(log)
}
return rlp.Encode(w, []interface{}{r.PostState, r.CumulativeGasUsed, r.Bloom, r.TxHash, r.ContractAddress, logs, r.GasUsed})
return rlp.Encode(w, []interface{}{r.PostState, r.Failed, r.CumulativeGasUsed, r.Bloom, r.TxHash, r.ContractAddress, logs, r.GasUsed})
}

// DecodeRLP implements rlp.Decoder, and loads both consensus and implementation
// fields of a receipt from an RLP stream.
func (r *ReceiptForStorage) DecodeRLP(s *rlp.Stream) error {
var receipt struct {
PostState []byte
Failed bool
CumulativeGasUsed *big.Int
Bloom Bloom
TxHash common.Hash
Expand All @@ -163,7 +177,7 @@ func (r *ReceiptForStorage) DecodeRLP(s *rlp.Stream) error {
return err
}
// Assign the consensus fields
r.PostState, r.CumulativeGasUsed, r.Bloom = receipt.PostState, receipt.CumulativeGasUsed, receipt.Bloom
r.PostState, r.Failed, r.CumulativeGasUsed, r.Bloom = receipt.PostState, receipt.Failed, receipt.CumulativeGasUsed, receipt.Bloom
r.Logs = make([]*Log, len(receipt.Logs))
for i, log := range receipt.Logs {
r.Logs[i] = (*Log)(log)
Expand Down
6 changes: 5 additions & 1 deletion core/vm/evm.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas
evm.Transfer(evm.StateDB, caller.Address(), to.Address(), value)

// initialise a new contract and set the code that is to be used by the
// E The contract is a scoped evmironment for this execution context
Copy link
Member

Choose a reason for hiding this comment

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

what a great typo - love this word!-)

// E The contract is a scoped environment for this execution context
// only.
contract := NewContract(caller, to, value, gas)
contract.SetCallCode(&addr, evm.StateDB.GetCodeHash(addr), evm.StateDB.GetCode(addr))
Expand Down Expand Up @@ -351,6 +351,10 @@ func (evm *EVM) Create(caller ContractRef, code []byte, gas uint64, value *big.I
contract.UseGas(contract.Gas)
}
}
// Assign err if contract code size exceeds the max while the err is still empty.
if maxCodeSizeExceeded && err == nil {
err = errMaxCodeSizeExceeded
}
return ret, contractAddr, contract.Gas, err
}

Expand Down
2 changes: 1 addition & 1 deletion core/vm/instructions.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ var (
errWriteProtection = errors.New("evm: write protection")
errReturnDataOutOfBounds = errors.New("evm: return data out of bounds")
errExecutionReverted = errors.New("evm: execution reverted")
errMaxCodeSizeExceeded = errors.New("evm: max code size exceeded")
)

func opAdd(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
Expand Down Expand Up @@ -619,7 +620,6 @@ func opCall(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack *Sta
if value.Sign() != 0 {
gas += params.CallStipend
}

ret, returnGas, err := evm.Call(contract, address, args, gas, value)
if err != nil {
stack.push(new(big.Int))
Expand Down
5 changes: 3 additions & 2 deletions eth/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -523,7 +523,8 @@ func (api *PrivateDebugAPI) TraceTransaction(ctx context.Context, txHash common.

// Run the transaction with tracing enabled.
vmenv := vm.NewEVM(context, statedb, api.config, vm.Config{Debug: true, Tracer: tracer})
ret, gas, err := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(tx.Gas()))
// TODO utilize failed flag
ret, gas, _, err := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(tx.Gas()))
if err != nil {
return nil, fmt.Errorf("tracing failed: %v", err)
}
Expand Down Expand Up @@ -570,7 +571,7 @@ func (api *PrivateDebugAPI) computeTxEnv(blockHash common.Hash, txIndex int) (co

vmenv := vm.NewEVM(context, statedb, api.config, vm.Config{})
gp := new(core.GasPool).AddGas(tx.Gas())
_, _, err := core.ApplyMessage(vmenv, msg, gp)
_, _, _, err := core.ApplyMessage(vmenv, msg, gp)
if err != nil {
return nil, vm.Context{}, nil, fmt.Errorf("tx %x failed: %v", tx.Hash(), err)
}
Expand Down
4 changes: 2 additions & 2 deletions eth/backend_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ func TestMipmapUpgrade(t *testing.T) {
chain, receipts := core.GenerateChain(params.TestChainConfig, genesis, db, 10, func(i int, gen *core.BlockGen) {
switch i {
case 1:
receipt := types.NewReceipt(nil, new(big.Int))
receipt := types.NewReceipt(nil, false, new(big.Int))
receipt.Logs = []*types.Log{{Address: addr}}
gen.AddUncheckedReceipt(receipt)
case 2:
receipt := types.NewReceipt(nil, new(big.Int))
receipt := types.NewReceipt(nil, false, new(big.Int))
receipt.Logs = []*types.Log{{Address: addr}}
gen.AddUncheckedReceipt(receipt)
}
Expand Down
10 changes: 5 additions & 5 deletions eth/filters/filter_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import (
)

func makeReceipt(addr common.Address) *types.Receipt {
receipt := types.NewReceipt(nil, new(big.Int))
receipt := types.NewReceipt(nil, false, new(big.Int))
receipt.Logs = []*types.Log{
{Address: addr},
}
Expand Down Expand Up @@ -145,7 +145,7 @@ func TestFilters(t *testing.T) {
var receipts types.Receipts
switch i {
case 1:
receipt := types.NewReceipt(nil, new(big.Int))
receipt := types.NewReceipt(nil, false, new(big.Int))
receipt.Logs = []*types.Log{
{
Address: addr,
Expand All @@ -155,7 +155,7 @@ func TestFilters(t *testing.T) {
gen.AddUncheckedReceipt(receipt)
receipts = types.Receipts{receipt}
case 2:
receipt := types.NewReceipt(nil, new(big.Int))
receipt := types.NewReceipt(nil, false, new(big.Int))
receipt.Logs = []*types.Log{
{
Address: addr,
Expand All @@ -165,7 +165,7 @@ func TestFilters(t *testing.T) {
gen.AddUncheckedReceipt(receipt)
receipts = types.Receipts{receipt}
case 998:
receipt := types.NewReceipt(nil, new(big.Int))
receipt := types.NewReceipt(nil, false, new(big.Int))
receipt.Logs = []*types.Log{
{
Address: addr,
Expand All @@ -175,7 +175,7 @@ func TestFilters(t *testing.T) {
gen.AddUncheckedReceipt(receipt)
receipts = types.Receipts{receipt}
case 999:
receipt := types.NewReceipt(nil, new(big.Int))
receipt := types.NewReceipt(nil, false, new(big.Int))
receipt.Logs = []*types.Log{
{
Address: addr,
Expand Down
Loading