From c251a14be8335e0286932794be895940c20e8c6e Mon Sep 17 00:00:00 2001 From: spooktheducks Date: Wed, 11 Mar 2020 22:24:49 -0500 Subject: [PATCH 1/2] Fix bad geth log decoding --- core/eth/contracts/FluxAggregator.go | 10 ++-- core/eth/contracts/FluxAggregator_test.go | 60 +++++++++++++++++++ core/eth/geth_copied.go | 6 ++ core/services/fluxmonitor/flux_monitor.go | 2 +- .../services/testdata/answer_updated_log.json | 22 +++++++ core/services/testdata/new_round_log.json | 2 +- .../testdata/round_details_updated_log.json | 23 +++++++ 7 files changed, 118 insertions(+), 7 deletions(-) create mode 100644 core/services/testdata/answer_updated_log.json create mode 100644 core/services/testdata/round_details_updated_log.json diff --git a/core/eth/contracts/FluxAggregator.go b/core/eth/contracts/FluxAggregator.go index 03fda0ee6c1..289a5b59336 100644 --- a/core/eth/contracts/FluxAggregator.go +++ b/core/eth/contracts/FluxAggregator.go @@ -41,16 +41,16 @@ type LogNewRound struct { type LogRoundDetailsUpdated struct { PaymentAmount *big.Int - MinAnswerCount *big.Int - MaxAnswerCount *big.Int - RestartDelay *big.Int - Timeout *big.Int + MinAnswerCount uint32 + MaxAnswerCount uint32 + RestartDelay uint32 + Timeout uint32 Address common.Address } type LogAnswerUpdated struct { Current *big.Int - RoundID *big.Int + RoundId *big.Int Timestamp *big.Int Address common.Address } diff --git a/core/eth/contracts/FluxAggregator_test.go b/core/eth/contracts/FluxAggregator_test.go index 9406e4f65a7..1a05bb26153 100644 --- a/core/eth/contracts/FluxAggregator_test.go +++ b/core/eth/contracts/FluxAggregator_test.go @@ -249,3 +249,63 @@ func TestFluxAggregatorClient_LatestSubmission(t *testing.T) { }) } } + +func TestFluxAggregatorClient_DecodesLogs(t *testing.T) { + fa, err := contracts.NewFluxAggregator(nil, common.Address{}) + require.NoError(t, err) + + newRoundLogRaw := cltest.LogFromFixture(t, "../../services/testdata/new_round_log.json") + var newRoundLog contracts.LogNewRound + err = fa.UnpackLog(&newRoundLog, "NewRound", newRoundLogRaw) + require.NoError(t, err) + require.Equal(t, int64(1), newRoundLog.RoundId.Int64()) + require.Equal(t, common.HexToAddress("f17f52151ebef6c7334fad080c5704d77216b732"), newRoundLog.StartedBy) + require.Equal(t, int64(15), newRoundLog.StartedAt.Int64()) + + type BadLogNewRound struct { + RoundID *big.Int + StartedBy common.Address + StartedAt *big.Int + } + var badNewRoundLog BadLogNewRound + err = fa.UnpackLog(&badNewRoundLog, "NewRound", newRoundLogRaw) + require.Error(t, err) + + answerUpdatedLogRaw := cltest.LogFromFixture(t, "../../services/testdata/answer_updated_log.json") + var answerUpdatedLog contracts.LogAnswerUpdated + err = fa.UnpackLog(&answerUpdatedLog, "AnswerUpdated", answerUpdatedLogRaw) + require.NoError(t, err) + require.Equal(t, int64(1), answerUpdatedLog.Current.Int64()) + require.Equal(t, int64(2), answerUpdatedLog.RoundId.Int64()) + require.Equal(t, int64(3), answerUpdatedLog.Timestamp.Int64()) + + type BadLogAnswerUpdated struct { + Current *big.Int + RoundID *big.Int + Timestamp *big.Int + } + var badAnswerUpdatedLog BadLogAnswerUpdated + err = fa.UnpackLog(&badAnswerUpdatedLog, "AnswerUpdated", answerUpdatedLogRaw) + require.Error(t, err) + + roundDetailsUpdatedLogRaw := cltest.LogFromFixture(t, "../../services/testdata/round_details_updated_log.json") + var roundDetailsUpdatedLog contracts.LogRoundDetailsUpdated + err = fa.UnpackLog(&roundDetailsUpdatedLog, "RoundDetailsUpdated", roundDetailsUpdatedLogRaw) + require.NoError(t, err) + require.Equal(t, int64(1), roundDetailsUpdatedLog.PaymentAmount.Int64()) + require.Equal(t, uint32(2), roundDetailsUpdatedLog.MinAnswerCount) + require.Equal(t, uint32(3), roundDetailsUpdatedLog.MaxAnswerCount) + require.Equal(t, uint32(4), roundDetailsUpdatedLog.RestartDelay) + require.Equal(t, uint32(5), roundDetailsUpdatedLog.Timeout) + + type BadLogRoundDetailsUpdated struct { + Paymentamount *big.Int + MinAnswerCount uint32 + MaxAnswerCount uint32 + RestartDelay uint32 + Timeout uint32 + } + var badRoundDetailsUpdatedLog BadLogRoundDetailsUpdated + err = fa.UnpackLog(&badRoundDetailsUpdatedLog, "RoundDetailsUpdated", roundDetailsUpdatedLogRaw) + require.Error(t, err) +} diff --git a/core/eth/geth_copied.go b/core/eth/geth_copied.go index 67cd8699478..15a0c7f0880 100644 --- a/core/eth/geth_copied.go +++ b/core/eth/geth_copied.go @@ -47,6 +47,12 @@ func parseTopics(out interface{}, fields abi.Arguments, topics []common.Hash) er if !arg.Indexed { return errors.New("non-indexed field in topic reconstruction") } + + _, exists := reflect.TypeOf(out).Elem().FieldByName(capitalise(arg.Name)) + if !exists { + return errors.Errorf(`can't find matching struct field for log "%T", field "%v" (expected "%v")`, out, arg.Name, capitalise(arg.Name)) + } + field := reflect.ValueOf(out).Elem().FieldByName(capitalise(arg.Name)) // Try to parse the topic back into the fields based on primitive types diff --git a/core/services/fluxmonitor/flux_monitor.go b/core/services/fluxmonitor/flux_monitor.go index c0fa9441f99..66259c596ce 100644 --- a/core/services/fluxmonitor/flux_monitor.go +++ b/core/services/fluxmonitor/flux_monitor.go @@ -494,7 +494,7 @@ func (p *PollingDeviationChecker) respondToAnswerUpdatedLog(log contracts.LogAns } func (p *PollingDeviationChecker) respondToRoundDetailsUpdatedLog(log contracts.LogRoundDetailsUpdated) error { - p.roundTimeout = time.Duration(log.Timeout.Int64()) * time.Second + p.roundTimeout = time.Duration(log.Timeout) * time.Second return nil } diff --git a/core/services/testdata/answer_updated_log.json b/core/services/testdata/answer_updated_log.json new file mode 100644 index 00000000000..abb25800c8a --- /dev/null +++ b/core/services/testdata/answer_updated_log.json @@ -0,0 +1,22 @@ +{ + "jsonrpc": "2.0", + "method": "eth_subscription", + "params": { + "subscription": "0x4a8a4c0517381924f9838102c5a4dcb7", + "result": { + "logIndex": "0x0", + "transactionIndex": "0x0", + "transactionHash": "0x420de56323893bced814b83f16a94c8ef7f7b6f1e3920a11ec62733fcf82c730", + "blockHash": "0x5e3bd2cc97a68136cead922330e2ec27201420b3eff182875e388474079fcd9e", + "blockNumber": "0xa", + "address": "0x2fCeA879fDC9FE5e90394faf0CA644a1749d0ad6", + "data": "0x0000000000000000000000000000000000000000000000000000000000000003", + "topics": [ + "0x0559884fd3a460db3073b7fc896cc77986f16e378210ded43186175bf646fc5f", + "0x0000000000000000000000000000000000000000000000000000000000000001", + "0x0000000000000000000000000000000000000000000000000000000000000002" + ], + "type": "mined" + } + } +} diff --git a/core/services/testdata/new_round_log.json b/core/services/testdata/new_round_log.json index 1d74600150e..5d622fc3c45 100644 --- a/core/services/testdata/new_round_log.json +++ b/core/services/testdata/new_round_log.json @@ -10,7 +10,7 @@ "blockHash": "0x5e3bd2cc97a68136cead922330e2ec27201420b3eff182875e388474079fcd9e", "blockNumber": "0xa", "address": "0x2fCeA879fDC9FE5e90394faf0CA644a1749d0ad6", - "data": "0x0000000000000000000000000000000000000000000000000000000000000000", + "data": "0x000000000000000000000000000000000000000000000000000000000000000f", "topics": [ "0x0109fc6f55cf40689f02fbaad7af7fe7bbac8a3d2186600afc7d3e10cac60271", "0x0000000000000000000000000000000000000000000000000000000000000001", diff --git a/core/services/testdata/round_details_updated_log.json b/core/services/testdata/round_details_updated_log.json new file mode 100644 index 00000000000..00b4236b065 --- /dev/null +++ b/core/services/testdata/round_details_updated_log.json @@ -0,0 +1,23 @@ +{ + "jsonrpc": "2.0", + "method": "eth_subscription", + "params": { + "subscription": "0x4a8a4c0517381924f9838102c5a4dcb7", + "result": { + "logIndex": "0x0", + "transactionIndex": "0x0", + "transactionHash": "0x420de56323893bced814b83f16a94c8ef7f7b6f1e3920a11ec62733fcf82c730", + "blockHash": "0x5e3bd2cc97a68136cead922330e2ec27201420b3eff182875e388474079fcd9e", + "blockNumber": "0xa", + "address": "0x2fCeA879fDC9FE5e90394faf0CA644a1749d0ad6", + "data": "0x00000000000000000000000000000000000000000000000000000000000000040000000000000000000000000000000000000000000000000000000000000005", + "topics": [ + "0x56800c9d1ed723511246614d15e58cfcde15b6a33c245b5c961b689c1890fd8f", + "0x0000000000000000000000000000000000000000000000000000000000000001", + "0x0000000000000000000000000000000000000000000000000000000000000002", + "0x0000000000000000000000000000000000000000000000000000000000000003" + ], + "type": "mined" + } + } +} From c1a35b7a83f4ad5157e9696a35b48b92f42d01c8 Mon Sep 17 00:00:00 2001 From: spooktheducks Date: Thu, 12 Mar 2020 11:31:03 -0500 Subject: [PATCH 2/2] Add comment explaining how Geth translates Solidity field names to Go struct field names --- core/eth/geth_copied.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/core/eth/geth_copied.go b/core/eth/geth_copied.go index 15a0c7f0880..3f77939b3f8 100644 --- a/core/eth/geth_copied.go +++ b/core/eth/geth_copied.go @@ -48,6 +48,13 @@ func parseTopics(out interface{}, fields abi.Arguments, topics []common.Hash) er return errors.New("non-indexed field in topic reconstruction") } + // If Go structs aren't kept correctly in sync with log fields defined in Solidity, this error will be returned. + // The name convention is to remove underscores, capitalize all characters following them, and capitalize the + // first letter of the field: + // + // round_id => RoundId + // roundId => RoundId + // _roundId => RoundId _, exists := reflect.TypeOf(out).Elem().FieldByName(capitalise(arg.Name)) if !exists { return errors.Errorf(`can't find matching struct field for log "%T", field "%v" (expected "%v")`, out, arg.Name, capitalise(arg.Name))