LF-Decentralized-Trust-labs / paladin

Programmable privacy for EVM
https://lf-decentralized-trust-labs.github.io/paladin
Apache License 2.0
15 stars 6 forks source link

propagate public tx options #431

Closed hosie closed 1 week ago

hosie commented 1 week ago

When invoking a contract on the pente domain, if I try to submit 2 transactions in quick succession, I am seeing error

ERROR Error dispatching transaction: PD011812: Public transaction rejected: PD012216: Transaction reverted PenteInputNotAvailable

I believe the reason for this is that we are estimating the gas based on the current committed block but my first transaction has not been added to a block yet so the 2nd transaction revers because it attempts to spend the states produced by the first.

This highlights the need for a conversation about what the default approach should be for gas estimation for the domain. The domain specific contracts should be fairly predictable in terms of the gas usage so it is not clear to me that we should be estimating gas for every transaction.

In lieu of that conversation, in the meantime, this PR enables the PublicTxOptions ( including gas) to be specified on input to ptx_sendTransaction. It is already part of the input type for ptx_sendTransaction but we were just not propagating it.

peterbroadhurst commented 1 week ago

@hosie - I'm not sure I understand this one yet.

If the assembly is being done with the wrong StateContext that is a coordinator issue.

I'm happy to review the code instead of the comment, but I'm worried about doing so without understanding the situation we're in and what we're trying to acheive.

hosie commented 1 week ago

Once I understand what is going on here, I have 2 questions in my mind

a) Do we want to be estimating the gas for every transaction? I propose the answer is no - if user as provided an explicit gas in the input. That is what this PR fixes. Otherwise, for me its a 'maybe'. Feels like we could have a more optimum strategy for gas estimation but this needs more thought/ discussion so I've not included a proposal in code for this just yet.

b) If we do decide to estimate gas, are we making the call correctly. Maybe we could do a successful gasEstimation in this case if we specify a block number pending. I have it on my TODO list to try that in addition to the code changes proposed in this PR.

peterbroadhurst commented 1 week ago

Thank you - fully understand now.

a) Do we want to be estimating the gas for every transaction? I propose the answer is no

This seems like the immediately solution. Asking the base EVM to estimage gas for an un-executable transaction is never going to work.

b) If we do decide to estimate gas, are we making the call correctly.

The only way I can see a possibility to do gas estimation on chained transaction, if we really decided we need it, would be to either:

1) spin up an ephemeral EVM to do it. 2) batch the transactions together at the smart contract layer, so they are one transaction