-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feat: chainstore: implement TipSetBlockMessagesReceipts method #9186
Conversation
0baec3d
to
4541329
Compare
msgIdx := 0 | ||
// index or receipts in `out.Receipts` | ||
receiptIdx := 0 |
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.
use one counter
chain/store/messages.go
Outdated
@@ -300,3 +301,144 @@ func (cs *ChainStore) LoadSignedMessagesFromCids(ctx context.Context, cids []cid | |||
|
|||
return msgs, nil | |||
} | |||
|
|||
// TipSetBlockMessagesReceipts returns the blocks and messages in `ts` and their corresponding receipts from `pts` matching block order in tipset (`ts`). |
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 comment is backwards, will fix
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.
Went over offline, generally looks good. @magik6k you might want to take a look to make sure we want this and you like how it is
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.
nvm I need to look more closely
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 looks correct, but is fine code -- I still only have about 50% confidence. I don't think this is landable without a test.
// The Receipts are one-to-one with Messages index. | ||
type BlockMessageReceipts struct { | ||
Block *types.BlockHeader | ||
// Messages contained in 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.
This comment is misleading -- these are not the messages contained (included) in a block, but rather the subset of those messages that were executed in the context of pts
.
receiptIdx := 0 | ||
out[blkIdx] = &BlockMessageReceipts{ | ||
// block containing messages | ||
Block: pts.Blocks()[blkIdx], |
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.
Nit: I would get pts.Blocks()[blkIdx]
from the range
operation.
// Receipts contained in Block. | ||
Receipts []*types.MessageReceipt | ||
// MessageExectionIndex contains a mapping of Messages to their execution order in the tipset they were included. | ||
MessageExecutionIndex map[types.ChainMsg]int |
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.
Is this really the map you want? Can it be indexed by msg cid instead?
- TipSetBlockMessagesReceipts returns the blocks and messages in a tipset and their corresponding receipts from its parent tipset matching block order in tipset.
- define ParentMessageReceipts method on tipset type - spelling - sanity checks in TipSetBlockMessagesReceipts
a07ee7e
to
d7951ee
Compare
Roger. Is there any prior art here? I was hoping to find a test that exercises other methods in this file. Do you know of any? |
The testing framework to validate this code change is not yet in place, will close and revisit it perhaps at a later time. |
receipts from its parent tipset matching block order in tipset.
Additional Info
Will be consumed by lily
Checklist
Before you mark the PR ready for review, please make sure that:
<PR type>: <area>: <change being made>
fix: mempool: Introduce a cache for valid signatures
PR type
: fix, feat, INTERFACE BREAKING CHANGE, CONSENSUS BREAKING, build, chore, ci, docs,perf, refactor, revert, style, testarea
: api, chain, state, vm, data transfer, market, mempool, message, block production, multisig, networking, paychan, proving, sealing, wallet, deps