-
Notifications
You must be signed in to change notification settings - Fork 338
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
postage/events: pkg for postage events and syncing with events #934
Conversation
51992b4
to
5f65b51
Compare
|
||
var ( | ||
eventSignatures = []string{ | ||
"BatchCreated(bytes32,uint256,address,uint256)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
depth
is now uint8
eventSignatures = []string{ | ||
"BatchCreated(bytes32,uint256,address,uint256)", | ||
"BatchTopUp(bytes32,uint256)", | ||
"BatchDepthIncrease(bytes32,uint256", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same here about depth
. also missing )
|
||
// parse reifies the event log type as a struct | ||
// weakly unsafe in that if there is another event, nil is returned | ||
func parse(log types.Log) postage.Event { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this approach does not work as some event parameters are indexed (only appears as topics, not events). see this for reference: https://github.com/ethereum/go-ethereum/blob/master/accounts/abi/bind/base.go#L350.
|
||
// parse reifies the event log type as a struct | ||
// weakly unsafe in that if there is another event, nil is returned | ||
func parse(log types.Log) postage.Event { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at the very least we must check for Removed
here. Otherwise not only are we not going to catch the reorganisation, we'll apply the removed event twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Regarding the tests you can just create instances of types.Log
with the right topics (like here #934, except with the real topics if you want to test the parse function).
Reorg handling will be tricky. Functions here which take the block number should at least also take the block hash as the same block number later might refer to a different block but block hashes never change.
Another important point to consider is that if the socket connection to the client breaks and we open a new subscription from the last block we processed we will not get Removed
events even in case of reorgs of events still received on the old subscription. This will lead to an inconsistent batch database. The same applies on bee restarts for blocks which were already processed but reorganised during the down time.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The most solid approach to the event tracking would be to store the block hash for every block number whenever we receive a (non-removed) event through the subscription. Then on startup and whenever the subscription is remade we can check if we still agree with the node on the last processed event (by checking the block hash which also includes the receipt hash in its computation). Otherwise we can walk back through the events and find the first event that is no longer valid and revert all of the ones following it. Then we can subscribe again from the block after the last agreed event.
@ralph-pichler could you take over? ;) |
Please attach this one to an issue. If no issue exists -- make an issue. Thanks |
134fa9a
to
cbd4d4a
Compare
cbd4d4a
to
97b2a15
Compare
closed in favor of #1099 |
this incomplete PR implements the go side of postage events and syncing with these events as per #920
see AP https://hackmd.io/o8RGsNwZSN6IE0lkBAZCPw?both
known to be missing:
types.Log
events without ethclient calls on actual contracts.types.Log.Removed
field and/or 'n-block confirmation delay'