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

Fix bug in Go RPC client when unmarshaling contract events #496

Merged
merged 2 commits into from Nov 5, 2019
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -25,6 +25,7 @@ This changelog is a work in progress and may contain notes for versions which ha
### Bug fixes 🐞

- Improved the aggressiveness at which we permanently delete orders that have been flagged for removal. Previously we would wait for the cleanup job to handle this (once an hour), but that meant many removed orders would accumulate. We now prune them every 5 minutes. ([#471](https://github.com/0xProject/0x-mesh/pull/471))
- Fixed a bug in the Go RPC client which resulted in errors when receving order events with at least one contract event ([#496](https://github.com/0xProject/0x-mesh/pull/496)).

## v5.1.0-beta

@@ -9,6 +9,7 @@ import (

"github.com/0xProject/0x-mesh/ethereum/signer"
"github.com/0xProject/0x-mesh/ethereum/wrappers"
"github.com/0xProject/0x-mesh/zeroex/orderwatch/decoder"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/common/math"
gethsigner "github.com/ethereum/go-ethereum/signer/core"
@@ -74,11 +75,6 @@ const (
OSInvalidTakerAssetData
)

// ContractEventParameters is the parameters of a ContractEvent
type ContractEventParameters interface {
json.Marshaler
}

// ContractEvent is an event emitted by a smart contract
type ContractEvent struct {
BlockHash common.Hash
@@ -88,7 +84,18 @@ type ContractEvent struct {
IsRemoved bool
Address common.Address
Kind string
Parameters ContractEventParameters
Parameters interface{}

This comment has been minimized.

Copy link
@albrow

albrow Nov 5, 2019

Author Member

@fabioberger changed this back to interface{} because for some event types, we just use the default json.Marshal behavior (which is more efficient and saves us some lines of code).

}

type contractEventJSON struct {
BlockHash common.Hash
TxHash common.Hash
TxIndex uint
LogIndex uint
IsRemoved bool
Address common.Address
Kind string
Parameters json.RawMessage
}

// MarshalJSON implements a custom JSON marshaller for the ContractEvent type
@@ -120,11 +127,11 @@ type OrderEvent struct {
}

type orderEventJSON struct {
OrderHash string `json:"orderHash"`
SignedOrder *SignedOrder `json:"signedOrder"`
EndState string `json:"endState"`
FillableTakerAssetAmount string `json:"fillableTakerAssetAmount"`
ContractEvents []*ContractEvent `json:"contractEvents"`
OrderHash string `json:"orderHash"`
SignedOrder *SignedOrder `json:"signedOrder"`
EndState string `json:"endState"`
FillableTakerAssetAmount string `json:"fillableTakerAssetAmount"`
ContractEvents []*contractEventJSON `json:"contractEvents"`
}

// MarshalJSON implements a custom JSON marshaller for the OrderEvent type
@@ -157,10 +164,127 @@ func (o *OrderEvent) fromOrderEventJSON(orderEventJSON orderEventJSON) error {
if !ok {
return errors.New("Invalid uint256 number encountered for FillableTakerAssetAmount")
}
o.ContractEvents = orderEventJSON.ContractEvents
o.ContractEvents = make([]*ContractEvent, len(orderEventJSON.ContractEvents))
for i, eventJSON := range orderEventJSON.ContractEvents {
contractEvent, err := unmarshalContractEvent(eventJSON)
if err != nil {
return err
}
o.ContractEvents[i] = contractEvent
}
return nil
}

func unmarshalContractEvent(eventJSON *contractEventJSON) (*ContractEvent, error) {
event := &ContractEvent{
BlockHash: eventJSON.BlockHash,
TxHash: eventJSON.TxHash,
TxIndex: eventJSON.TxIndex,
LogIndex: eventJSON.LogIndex,
IsRemoved: eventJSON.IsRemoved,
Address: eventJSON.Address,
Kind: eventJSON.Kind,
}

switch eventJSON.Kind {
case "ERC20TransferEvent":
var parameters decoder.ERC20TransferEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "ERC20ApprovalEvent":
var parameters decoder.ERC20ApprovalEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "ERC721TransferEvent":
var parameters decoder.ERC721TransferEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "ERC721ApprovalEvent":
var parameters decoder.ERC721ApprovalEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "ERC721ApprovalForAllEvent":
var parameters decoder.ERC721ApprovalForAllEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "ERC1155TransferSingleEvent":
var parameters decoder.ERC1155TransferSingleEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "ERC1155TransferBatchEvent":
var parameters decoder.ERC1155TransferBatchEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "ERC1155ApprovalForAllEvent":
var parameters decoder.ERC1155ApprovalForAllEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "WethWithdrawalEvent":
var parameters decoder.WethWithdrawalEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "WethDepositEvent":
var parameters decoder.WethDepositEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "ExchangeFillEvent":
var parameters decoder.ExchangeFillEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "ExchangeCancelEvent":
var parameters decoder.ExchangeCancelEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

case "ExchangeCancelUpToEvent":
var parameters decoder.ExchangeCancelUpToEvent
if err := json.Unmarshal(eventJSON.Parameters, &parameters); err != nil {
return nil, err
}
event.Parameters = parameters

default:
return nil, fmt.Errorf("unknown event kind: %s", eventJSON.Kind)
}

return event, nil
}

// OrderEventEndState enumerates all the possible order event types. An OrderEventEndState describes the
// end state of a 0x order after revalidation
type OrderEventEndState string
@@ -8,6 +8,7 @@ import (
"testing"

"github.com/0xProject/0x-mesh/constants"
"github.com/0xProject/0x-mesh/zeroex/orderwatch/decoder"
"github.com/ethereum/go-ethereum/common"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
@@ -58,7 +59,22 @@ func TestMarshalUnmarshalOrderEvent(t *testing.T) {
SignedOrder: signedOrder,
EndState: ESOrderAdded,
FillableTakerAssetAmount: big.NewInt(2000),
ContractEvents: []*ContractEvent{},
ContractEvents: []*ContractEvent{

This comment has been minimized.

Copy link
@albrow

albrow Nov 5, 2019

Author Member

@fabioberger we didn't catch this bug because we never tried marshaling/unmarshaling an OrderEvent with a non-empty ContractEvents field.

{
BlockHash: common.HexToHash("0x3fcd58a6613265e2b0deba902d7ff693f330a0af6e5b04805b44bbffd8a415d4"),
TxHash: common.HexToHash("0x3fcd58a6613265e2b0deba902d7ff693f330a0af6e5b04805b44bbffd8a415d5"),
TxIndex: 42,
LogIndex: 1337,
IsRemoved: true,
Address: common.HexToAddress("0x1dc4c1cefef38a777b15aa20260a54e584b16c49"),
Kind: "ERC20TransferEvent",
Parameters: decoder.ERC20TransferEvent{
From: common.HexToAddress("0x1dc4c1cefef38a777b15aa20260a54e584b16c50"),
To: common.HexToAddress("0x1dc4c1cefef38a777b15aa20260a54e584b16c51"),
Value: big.NewInt(120),
},
},
},
}

buf := &bytes.Buffer{}
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.