remove UseInboxContract and optimize getTxSucceed

[blockchaindevsh]

As disccussed previously, we want to remove the UseInboxContract option and always check tx status.

This PR also optimizes getTxSucceed to return a smaller map by filtering with batch inbox address.

[qizhou]

LGTM. Please wait for @qzhodl 's review before merging.

[qzhodl]

LGTM. Please wait for @qzhodl 's review before merging.

Sorry, but I may not have the full context of this change or the related discussion. Could you clarify the reasoning behind this change? Have there been any assumption changes, such as always assuming that the batch inbox is now a smart contract?

[blockchaindevsh]

LGTM. Please wait for @qzhodl 's review before merging.

Sorry, but I may not have the full context of this change or the related discussion. Could you clarify the reasoning behind this change? Have there been any assumption changes, such as always assuming that the batch inbox is now a smart contract?

There's really no good way to check if it's an EOA, think of this case:

Initially there's no code for an address A, and we think it's an EOA.

But in theory it can be a CREATE/CREATE2 address, and in the next block, there can be a contract deployed at address A.

So instead of checking if there's code at address A every block, we decided to check tx status each time.

[blockchaindevsh]

@qizhou I made another change to getTxSucceed for optimization in this PR, since the diff is small and to avoid conflict if done in another PR.

[qizhou]

@qizhou I made another change to getTxSucceed for optimization in this PR, since the diff is small and to avoid conflict if done in another PR.

Could you please update the title and description accordingly? Current title and description seem to be misleading.

[blockchaindevsh]

@qizhou I made another change to getTxSucceed for optimization in this PR, since the diff is small and to avoid conflict if done in another PR.

Could you please update the title and description accordingly? Current title and description seem to be misleading.

Done.

[qizhou]

LGTM. Please wait for @qzhodl 's review before merging.

Sorry, but I may not have the full context of this change or the related discussion. Could you clarify the reasoning behind this change? Have there been any assumption changes, such as always assuming that the batch inbox is now a smart contract?

There's really no good way to check if it's an EOA, think of this case:

Initially there's no code for an address A, and we think it's an EOA.

But in theory it can be a CREATE/CREATE2 address, and in the next block, there can be a contract deployed at address A.

So instead of checking if there's code at address A every block, we decided to check tx status each time.

I guess you haven't answered Qiang's question directly. My understanding is that the PR simplifies the smart contract inbox support (removing UseInboxContract flag) while still supports EOA if the inbox has no code.

[qzhodl]

If the inbox is changed from an EOA to a smart contract, would we need to restart the batch, or is the system able to handle this transition smoothly?

[blockchaindevsh]

This is a great question!

(Note we don't consider dynamic inbox for this PR, instead we assume it's a fixed address that never changes, but the content can change for the reason here.)

I think in theory we can not reliably cache the result, but in reality the chance it actually happens is small, so maybe it's good enough if we just clear the cache(set inboxIsEOA to nil) when a transaction failure is spotted afterwards?

[blockchaindevsh]

@qzhodl Please take another look at the latest commit, which applies the above change.