Prioritize randomness transactions#1
Prioritize randomness transactions#1GabrielMartinezRodriguez wants to merge 4 commits intohappy-current-versionfrom
Conversation
GabrielMartinezRodriguez
changed the title
| delete(pendingBlobTxs, randomnessOwner) | ||
| } | ||
|
|
||
| if len(privilegedPlainTxs) > 0 || len(privilegedBlobTxs) > 0 { |
There was a problem hiding this comment.
do we need to "prioritize" blob type transactions too?
There was a problem hiding this comment.
Actually, it is not needed, but if you read the code below, you’ll see that they split the transaction into two groups: one for local and one for remote. In each of them, they process the blobs for every address together with the local ones. I can change it, but in my opinion, it slightly breaks the pattern. What do you think?
There was a problem hiding this comment.
Yeah I think leave as you have implemented for now
| for attempt := 1; attempt <= attempts; attempt++ { | ||
| // Gas prices: priority < non-priority (reversed from normal scenario) | ||
| priorityGasPrice := big.NewInt(params.InitialBaseFee * 5) | ||
| nonPriorityGasPrice := big.NewInt(params.InitialBaseFee * 50) |
There was a problem hiding this comment.
this is cool - i also tried changing the nonPriorityGasPrice to be lower than the priorityGasPrice and the test still passes which makes sense, the transactions are ordered in priority regardless of gasPrice (which is what we want), though i think it makes most sense to test for this case
| } | ||
| } | ||
|
|
||
| if minedBlock == nil { |
| } | ||
| } | ||
|
|
||
| func newCustomTestWorkerBackend(t *testing.T, chainConfig *params.ChainConfig, engine *clique.Clique, db ethdb.Database, chain *core.BlockChain) *testWorkerBackend { |
There was a problem hiding this comment.
i think we dont need params t and engine
| Config: &config, | ||
| Timestamp: 1, | ||
| ExtraData: extra, | ||
| Alloc: core.GenesisAlloc{ |
There was a problem hiding this comment.
my vscode tells me core.GenesisAlloc is deprecated, changing to the suggested types.GenesisAlloc works in place
|
|
||
| priorityAddressHash := common.BytesToHash(common.LeftPadBytes(priorityAddress.Bytes(), 32)) | ||
|
|
||
| otherKey, _ := crypto.GenerateKey() |
There was a problem hiding this comment.
nit: renaming suggestion: nonPriorityKey and nonPriorityAddress
| } | ||
|
|
||
| // Create a worker | ||
| w := newWorker(priorityConfig, &config, engine, backend, new(event.TypeMux), nil, false) |
There was a problem hiding this comment.
event.TypeMux also marked as deprecated, but the suggested replacement doesnt work seemingly without changing the newWorker signature so just leave as is
| time.Sleep(1 * time.Second) | ||
| continue | ||
| } | ||
| minedBlock = block |
There was a problem hiding this comment.
fwiw blockchain.InsertChain() also works here but i dont think its necessary.
2 participants
This PR aims to modify op-geth so that the randomness service transaction is always executed first. To accomplish this, the PR adds a new command parameter to specify the randomness service contract. Then, during block building, the owner account of the randomness contract is checked in order to prioritize transactions from that address