-
Notifications
You must be signed in to change notification settings - Fork 17
adding a new function for reconciling confirmations map in the connector interface #148
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
base: main
Are you sure you want to change the base?
adding a new function for reconciling confirmations map in the connector interface #148
Conversation
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
7bc7bb8
to
07127ff
Compare
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
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.
A few thoughts
pkg/ffcapi/api.go
Outdated
// the key of the map is the hash of the first block (tx block) that contains the transaction hash | ||
// the value of the map is an array of blocks, the first block is the tx block | ||
// the rest of the blocks are blocks inside the range of the target confirmation count | ||
// gap is allowed between the tx block and the second block |
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.
Why is this allowed?
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.
shouldn't this just be an implementation detail from the connector?
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.
Could argue it's a connector implementation details. Will remove that from the comment
Signed-off-by: Chengxuan Xing <chengxuan.xing@kaleido.io>
Exposing a method to allow confirmation map building using the in-memory canonical chain in the connector for efficient confirmation calculation of any given tx hash.
The new approach is similar to the existing confirmation manager, but with the following differences:
Additionally, there is a significant design difference when it comes to confirming a transaction that is inside a block no longer in the in-memory chain: the existing confirmation manager prioritizes confirmation details over confirmation latency and resource usage, whereas the new ReconcileConfirmationsForTransaction follows an instant confirmation approach.
More details in the firefly-evmconnect PR: hyperledger/firefly-evmconnect#174
A new confirmation map struct is introduced to capture all the potential confirmation queues (due to forking) of a single transaction hash. The confirmation map struct needs to be stored and passed back to the
ReconcileConfirmationsForTransaction
by the consumer in order to persist confirmation queues that are no longer on the latest canonical chain.