init #1
init #1
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
| uint256 _amount, | ||
| uint32 _minGasLimit, | ||
| bytes calldata _extraData | ||
| ) public payable { |
There was a problem hiding this comment.
The user may compose malicious tx for the bot runner to finalize, and these limitations can't be hardcoded in the contracts. So we should set filter for events to handle:
_l2Tokenin known list_minGasLimitcan't be bigger than known value. In practice, we'll set a fixed value in the front end_extraDatashould be empty
txns from bridge UI will match the filter while custom txn sent directly by users may not.
There was a problem hiding this comment.
Hi @owen-reorg I have created another PR #8
With #8, I think it can handle malicious cases alraedy.
Then I think this suggesting filter may be not necessary for a malicious withdrawal, since the contract will return a revert error when proving or finalizing. Afterward, we will record the revert-error and skip the reverted withdrawals later.
| LogIndex int `gorm:"type:integer;not null;uniqueIndex:idx_l2_contract_events_block_hash_log_index_key,priority:2"` | ||
| EventSignature string `gorm:"type:varchar(256);not null"` | ||
| Proven bool `gorm:"type:boolean;not null;default:false"` | ||
| Finalized bool `gorm:"type:boolean;not null;default:false"` |
There was a problem hiding this comment.
If a withdrawal txn is not expected, we may need to ignore it. I suggest to add a field for this.
There was a problem hiding this comment.
Another PR has been created to apply this suggestion #8
| name = "db_name" | ||
|
|
||
| [l1-contracts] | ||
| address-manager = "0x1111111111111111111111111111111111111111" |
There was a problem hiding this comment.
It's determined for both testnet and mainnet.
- Remove unnecessary configurations.
- List both testnet and mainnet configurations here. Use testnet as the default and comment out the mainnet configuration.
| return fmt.Errorf("hashMesaageHash err: %v", err) | ||
| } | ||
|
|
||
| l2OutputIndex, outputProposal, err := b.getL2OutputAfter(l2BlockNumber) |
There was a problem hiding this comment.
I believe we can use the latest L2 block number in L2OutputOracle. Can you please give it a try? If we use an earlier block, the performance of getProof will be worse. Additionally, nodes running in certain modes may remove the early trie data, making it impossible to obtain the proof.
| b.L1Contracts.OptimismPortalProxy, | ||
| b.L1Client, | ||
| ) | ||
| signedTx, err := optimismPortalTransactor.ProveWithdrawalTransaction( |
There was a problem hiding this comment.
will it dry run before send?
what if the tx fails?
what if the tx pending for a long time?
There was a problem hiding this comment.
-
It will estimate gas before sending. Eth_estimateGas can be considered a dry run.
-
No retry for tx failures. These unproven withdrawals will be rescanned by the processor goroutine and retried to prove.
-
As (2) says, I think failed transactions will be retried by the processor goroutine.
I am finding a way to enhance the transaction management, please touch me if you have some ideas.
This comment was marked as resolved.
This comment was marked as resolved.
1. contracts: transfer fee via call 2. cmd: git add run.go 3. core: add SignerConfig for signer and gas price 4. core: reduce duplicated code in ProveWithdrawalTransaction and FinalizeWithdrawalTransaction 5. core: fix type definition ContractAddress 6. ci: add workflow build.yaml
Yes, the fun fact is that I make some updates at ddfaf66 according to the below review suggestions:
|
| result := db.Order("number desc").Last(&l2ScannedBlock) | ||
| if result.Error != nil { | ||
| if errors.Is(result.Error, gorm.ErrRecordNotFound) { | ||
| db.Create(&l2ScannedBlock) |
There was a problem hiding this comment.
shall we set a start number in config instead of start from block #0?
| continue | ||
| } else if len(unprovens) > 0 { | ||
| for _, unproven := range unprovens { | ||
| if unproven.BlockTime+cfg.Misc.ProposeTimeWindow < time.Now().Unix() { |
There was a problem hiding this comment.
Normally too deep nested code is hard to read. I suggest rewrite it by handle earlier with esle condition.
E.g.
if unproven.BlockTime+cfg.Misc.ProposeTimeWindow >= time.Now().Unix() {
continue
}
original_if_code_block()
There was a problem hiding this comment.
Another PR has been created to refactor this function, to make it more readable. PTAL #7
No description provided.