Revert "fix: allow CONTRACT_NOT_FOUND error for starknet_estimateMessageFee"#352
Open
thiagodeev wants to merge 1 commit intostarkware-libs:release/v0.10.1-rc.0from
Conversation
a4e4012 to
1fe99ec
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context of the PR. Some messages I sent on Slack:
The
CONTRACT_NOT_FOUNDerror is currently returned by these methods:starknet_call(hascontract_addressparam in the fn_call)starknet_estimateFeestarknet_estimateMessageFeestarknet_getClassHashAt(hascontract_addressparam)starknet_getClassAt(hascontract_addressparam)starknet_getNonce(hascontract_addressparam)starknet_getStorageAt(hascontract_addressparam)The methods that contain the contract_address param are easy to understand; there's only a single point of cause for a contract to be "not found": the contract_address contract. But what about
starknet_estimateFeeandstarknet_estimateMessageFee? How to know when to return theCONTRACT_NOT_FOUNDerror?Should it be when the sender address wasn't found? Or the contract specified in the calldata? Or when doing an inner call to another contract that wasn't found (also important in the starknet_call method)?
IMHO, for this case, we don't need to add a new error type. All the cases are being addressed through the current errors in both starknet_estimateFee and starknet_estimateMessageFee methods. For e.g., in the starknet_estimateFee:
BLOCK_NOT_FOUNDor-32602 Invalid params-32602 Invalid paramsTRANSACTION_EXECUTION_ERRORsince it was found while executing the given txn. Invalid sender address? returnTRANSACTION_EXECUTION_ERRORspecifying it. Invalid signature?TRANSACTION_EXECUTION_ERROR. Invalid sender address in an inner call?TRANSACTION_EXECUTION_ERRORhandle inner call cases with the innerCONTRACT_EXECUTION_ERROR_INNERtype (same for thestarknet_estimateMessageFeemethod, as we have theCONTRACT_ERRORtype.If we try to create custom errors for every field, we will end up with too many error types (like
SENDER_ADDRESS_NOT_FOUND,INVALID_NONCE,INVALID_TXN_TYPE,INVALID_L1_ADDRESS, etc...), not to mention some of them would have to support inner calls like theCONTRACT_EXECUTION_ERROR_INNER, since they could be triggered on it.Therefore, I believe we should just remove the
CONTRACT_NOT_FOUNDerror from these methods.Differently from methods like
starknet_getClassHashAt, which has a specificcontract_addressparameter, every occurrence of a contract address in these methods is through inner parameters (inside theBROADCASTED_TXNarray in thestarknet_estimateFee, and inside theMSG_FROM_L1in thestarknet_estimateMessageFee). Why return a specific error for a single inner parameter, and not for the others?