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

Allow defining only a reader #3

Open
mratsim opened this issue May 1, 2019 · 4 comments
Open

Allow defining only a reader #3

mratsim opened this issue May 1, 2019 · 4 comments

Comments

@mratsim
Copy link
Contributor

mratsim commented May 1, 2019

For the state tests I'm defining only a Bitfield reader but it apparently triggered issues:

WRN 2019-04-30 14:36:39-06:00 Error while handling RLPx message          topics="rlpx" tid=6523 err="\nAsync traceback:\n  beacon_node.nim(794)         beacon_node\n  beacon_node.nim(717)         start\n  beacon_node.nim(690)         run\n  asyncloop.nim(822)           ru
    runForever\n  asyncloop.nim(266)           poll\n  asyncmacro2.nim(39)          dispatchMessages_continue\n  rlpx.nim(533)                dispatchMessagesIter\n  rlpx.nim(266)                invokeThunk\n  asyncmacro2.nim(306)         emit_thunk\n  asyncmacro2.nim(36)
im(36)          emit_thunk_continue\n  rlpx.nim(802)                emit_thunkIter\n  asyncmacro2.nim(306)         emit\n  asyncmacro2.nim(36)          emit_continue\n  gossipsub_protocol.nim(65)   emitIter\n  serialization.nim(65)        :anonymous\n  serialization.nim(4
.nim(46)        readValue\n  reader.nim(181)              readValue\n  object_serialization.nim(90) :anonymous\n  bitfield.nim(17)             readValue\n  serialization.nim(46)        readValue\n  reader.nim(111)              readValue\n  reader.nim(91)               req
   requireToken\n  reader.nim(64)               raiseUnexpectedToken\n  #[\n    beacon_node.nim(794)         beacon_node\n    beacon_node.nim(717)         start\n    beacon_node.nim(690)         run\n    asyncloop.nim(822)           runForever\n    asyncloop.nim(266)
)           poll\n    asyncmacro2.nim(39)          dispatchMessages_continue\n    rlpx.nim(533)                dispatchMessagesIter\n    rlpx.nim(266)                invokeThunk\n    asyncmacro2.nim(306)         emit_thunk\n    asyncmacro2.nim(39)          emit_thunk_cont
k_continue\n    rlpx.nim(802)                emit_thunkIter\n    asyncfutures2.nim(377)       read\n  ]#\nException message: \nException type:" msg=emit peer=Node[127.0.0.1:50007] node=0

cc @arnetheduck

Those were solved by commenting out the reader in status-im/nimbus-eth2#259

This would be helpful for YAML serialization so that we can forego the writer implementation.

@arnetheduck
Copy link
Member

well, it stands to reason that when you introduce a custom reader, you also need a corresponding writer so that the serialization round-trips, no?

@mratsim
Copy link
Contributor Author

mratsim commented May 7, 2019

Is it really necessary? For configuration file like .ini, .toml or in your case for .yaml tests we don't need to produce them.

From a time-to-market point-of-view, it saves half the work (differences between parsing and generation aside).

@arnetheduck
Copy link
Member

well, it's necessary if you write :) that code failed because it was calling that serializer and trying to read back on the other end (broadcasts etc) ☯️

@mratsim
Copy link
Contributor Author

mratsim commented May 7, 2019

Mmmh right I think I conflated 2 issues:

  1. Reading and writing types - the Bitfield case.
  2. Reading and writing format (i.e. de/serialization of yaml, ini, toml, ...)

We had a writer missing on case 1 leading to a bug and I assumed that for case 2 (format) we would have the same issue.

So the question is "can we implement only a reading for a serialization format without the corresponding writer?", if yes we can close the issue.

In short the context is wrong but the question is valid :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants