Limit enclave fed L1 receipts to obscuro-relevant tx #1495
Conversation
| @@ -125,8 +125,6 @@ type Enclave interface { | |||
| StreamL2Updates() (chan StreamL2UpdatesResponse, func()) | |||
| // DebugEventLogRelevancy returns the logs of a transaction | |||
| DebugEventLogRelevancy(hash gethcommon.Hash) (json.RawMessage, SystemError) | |||
|
|
|||
| Config() (*ObscuroEnclaveInfo, SystemError) | |||
There was a problem hiding this comment.
This method existed because there was some config (sequencerID and message bus address) that wasn't available to the host but we wanted to serve those details on obscuroscan API. Since I needed one of them I thought I might as well add both properties and remove the method.
There was a problem hiding this comment.
I wonder if it might still be useful in the HA setting.
Even as a sanity check to spot config problems early. Basically, when a host starts, or when an enclave is added, the host checks that the config matches the enclave
There was a problem hiding this comment.
I think we were planning to bake the static network level config into the enclave image which we sign. I agree something like that could be useful but I feel like we haven't reached our end-state of the config situation yet so kinda hard to predict.
Don't feel too strongly either way though, I can put it back if you prefer?
| @@ -125,10 +131,52 @@ func (p HostInputConfig) ToHostConfig() *HostConfig { | |||
|
|
|||
| // HostConfig contains the configuration used in the Obscuro host execution. Some fields are derived from the HostInputConfig. | |||
| type HostConfig struct { | |||
| ///// | |||
There was a problem hiding this comment.
This config was getting huge, I tried to group it logically but happy to revert if this isn't helpful. Hoped it might inspire someone to think about how our config should be structured :)
| @@ -44,13 +44,6 @@ func (m *blockMessageExtractor) Enabled() bool { | |||
| func (m *blockMessageExtractor) StoreCrossChainMessages(block *common.L1Block, receipts common.L1Receipts) error { | |||
| defer m.logger.Info("Block cross chain messages processed", log.BlockHashKey, block.Hash(), log.DurationKey, measure.NewStopwatch()) | |||
|
|
|||
| areReceiptsValid := common.VerifyReceiptHash(block, receipts) | |||
There was a problem hiding this comment.
We were verifying the receipt hash by reconstructing it using all the receipts but that check is not going to be feasible.
There was a problem hiding this comment.
how can we do it? Using Merkle branches?
There was a problem hiding this comment.
LLM has convinced me we could indeed provide a merkle proof for each receipt and that could give it high confidence the receipts are legit (so long as it can trust the receipt root hash).
Doesn't seem that hard to do but prefer to leave it out of this PR. Shall I leave a todo and create a GH issue?
There was a problem hiding this comment.
Actually would we be able to do it without having to request all the receipt data... I guess not. So maybe we need to resign ourselves to doing a receipt lookup for every transaction anyway? We need to have a think about this.
I'm going to raise another PR that caches the receipts because part of the data problem is re-lookups when things are re-submitted I think.
BedrockSquirrel
force-pushed
the
matt/only-submit-relevant-receipts
branch
from
e98a6d8 to
8124a4f
Compare
| @@ -125,8 +125,6 @@ type Enclave interface { | |||
| StreamL2Updates() (chan StreamL2UpdatesResponse, func()) | |||
| // DebugEventLogRelevancy returns the logs of a transaction | |||
| DebugEventLogRelevancy(hash gethcommon.Hash) (json.RawMessage, SystemError) | |||
|
|
|||
| Config() (*ObscuroEnclaveInfo, SystemError) | |||
There was a problem hiding this comment.
I wonder if it might still be useful in the HA setting.
Even as a sanity check to spot config problems early. Basically, when a host starts, or when an enclave is added, the host checks that the config matches the enclave
| @@ -44,13 +44,6 @@ func (m *blockMessageExtractor) Enabled() bool { | |||
| func (m *blockMessageExtractor) StoreCrossChainMessages(block *common.L1Block, receipts common.L1Receipts) error { | |||
| defer m.logger.Info("Block cross chain messages processed", log.BlockHashKey, block.Hash(), log.DurationKey, measure.NewStopwatch()) | |||
|
|
|||
| areReceiptsValid := common.VerifyReceiptHash(block, receipts) | |||
There was a problem hiding this comment.
how can we do it? Using Merkle branches?
Why this change is needed
To keep costs down (both API usage and latency) we only want to request receipts for transactions that the enclave cares about (currently just management contract and message bus).
What changes were made as part of this PR
PR checks pre-merging
Please indicate below by ticking the checkbox that you have read and performed the required
PR checks