bug: accounts are removed bypassing the execution

[bmuddha]

In chainlink, when LRU cache overflows it evicts accounts, which results in unsubscription, followed by account removal from accountsdb. This has several problems:

1. If someone is currently operating on that account, it will lead to a nasty data race and potential UB
2. Replication stream is completely unaware of the fact that accountsdb state has been changed, due to the lack of transaction and will result in a state mismatch with replica nodes.

We need to add a separate instruction to handle account closures, and do the operation properly via transaction.

magicblock-validator/magicblock-chainlink/src/chainlink/mod.rs

Line 232 in dac276f

fn subscribe_account_removals(

[thlorenz]

I have been aware of this theoretical issue which would happen in super rarely.
However I don't think evicting via a transaction would improve the failure case, nor is it necessary, due to this only applying to readonly accounts (more details below).

Therefore case 1 is not really an issue IMHO.

---

Case 1

Current Behavior

Let's look at what accounts are affected first:

1. Only undelegated accounts are ever removed
2. That means those accounts can only be used as readonly by other transactions
3. When a transaction arrives that needs this account to execute it is immediately promoted
4. As a result all other stale accounts would be removed before that account is removed as well

Thus it is super rare that an account that is needed by a tx that just came in would ever be removed while it is being scheduled.
In that case the transaction would fail, but could be retried by the client, i.e. it doesn't lead to an unresolvable problem nor does it have any security impact.

Since no transaction could ever write to this account (outside our internal clone/remove transactions) we would never run into a write access conflict. All that could happen (in super rare cases) is that an account that is required by a transaction was removed and the tx fails.

What the Fix would Do

Given that we already have no write access conflict I'm not sure what using a transaction for account eviction would improve here.
If we play this through we'd run into the same issue (super rarely).

1. TX accessing the account comes in and account is promoted
2. Account still ends up being evicted (with a proper transaction) before that outside transaction runs
3. Transaction still fails since the account is no longer present in our validator

The fact that we use a transaction does still not guarantee that the outside transaction depending on the account would run before the eviction.

Please LMK if I'm missing something here and/or what other improvement using a transaction would give us for this case.

---

Case 2

WRT replication I agree that we would keep this account in the bank which would not be an issue as long as the node is simply replicating, however if it switched over to become leader, we might see isues since now it would hold that account in the bank with potentially no subscription.

Also if an account got evicted in the leader and the tx fails there, but runs fine in the replidation node we could see a problem there.

So I agree that for this case we may have to adapt this solution.

LMK if you have other ideas after evaluating this context, otherwise I'll go ahead with the implementation.