Feat/runtime analysis errors #3234
Conversation
…t caught at runtime (but as a String, since they don't serialize)
…the transaction will not invalidate the block. Add testing for this gate.
| return Ok(receipt); | ||
| } else { | ||
| // prior to 2.1, this is not permitted in a block. | ||
| error!("Unexpected analysis error invalidating transaction: if included, this will invalidate a block"; |
There was a problem hiding this comment.
I think this should be warn! level -- error level logs should mean that something is wrong and that the operator of the node should be trying to take action to remedy it.
| auth.clone(), | ||
| TransactionPayload::new_smart_contract( | ||
| &"foo-impl".to_string(), | ||
| &runtime_checkerror_trait.to_string(), |
There was a problem hiding this comment.
Is this supposed to be runtime_checkerror_impl?
| let (fee, tx_receipt) = StacksChainState::process_transaction( | ||
| &mut conn, | ||
| &signed_test_trait_checkerror_tx, | ||
| false, | ||
| ) | ||
| .unwrap(); | ||
| assert_eq!(fee, 0); | ||
|
|
||
| assert!(tx_receipt.vm_error.is_some()); | ||
| let err_str = tx_receipt.vm_error.unwrap(); | ||
| assert!(err_str | ||
| .find("TypeValueError(OptionalType(TraitReferenceType(TraitIdentifier ") | ||
| .is_some()); |
There was a problem hiding this comment.
Can you add checks to this test for the following:
- In 2.1, the fee (which should be positive for the test to actually assert a change) is assessed against the payer's account, and the nonce is bumped for the failed transaction.
- In 2.05, the fee isn't assessed (again, it needs to be positive) against the payer's account, and the nonce isn't bumped for the failed transaction.
- In 2.1, the transaction is committed, but there's no state change. To do this, I think you'd need to invoke
(flip)at some point in thetestmethod before triggering the error. Then just assert thatfliphasn't changed after the tx.
Then, I think this needs an additional (very similar) battery of tests for the case where the "handled" error comes from a contract publish -- I think if you add a (test .foo-impl) invocation to the bottom of the contract publish, it'll create such a situation.
…dies in tests; test `contract-call?` within a smart contract for runtime checkerrors
Codecov Report
@@ Coverage Diff @@
## feat/bill-fee-before-tx-processing #3234 +/- ##
=======================================================================
+ Coverage 29.82% 84.75% +54.93%
=======================================================================
Files 276 276
Lines 221418 221917 +499
=======================================================================
+ Hits 66035 188091 +122056
+ Misses 155383 33826 -121557
Help us with your feedback. Take ten seconds to tell us how you rate us. |
See #3107, which this PR addresses. |
…e changes materialize when a transaction fails due to an analysis error found at runtime
This PR makes all
Error::Unchecked(..)Clarity errors encountered at runtime into runtime errors that do not invalidate the block. This feature is gated and only takes effect in Stacks 2.1.