Rework error names #712
Rework error names #712
Conversation
Fumuran
force-pushed
the
andrew-rework-error-naming
branch
from
7580f7b to
7593fa0
Compare
miden-tx/src/error.rs
Outdated
| // --- P2ID ERRORS -------------------------------------------------------------------------------- | ||
| const ERR_P2ID_EXPECTS_EXACTLY_1_NOTE_INPUT: u32 = 131074; // 131074 0x00020002 | ||
| const ERR_P2ID_MISMATCH_OF_TARGET_ACCOUNT_ADDR_AND_TRANSACTION_ADDR: u32 = 131075; // 131075 0x00020003 | ||
|
|
||
| // --- P2IDR ERRORS ------------------------------------------------------------------------------- | ||
| const ERR_P2IDR_CAN_BE_RECLAIMED_ONLY_BY_SENDER: u32 = 131077; // 131077 0x00020005 | ||
| const ERR_P2IDR_EXPECTS_EXACTLY_2_NOTE_INPUTS: u32 = 131076; // 131076 0x00020004 | ||
| const ERR_P2IDR_RECLAIM_BLOCK_HEIGHT_NOT_REACHED: u32 = 131078; // 131078 0x00020006 | ||
|
|
||
| // --- SWAP ERRORS -------------------------------------------------------------------------------- | ||
| const ERR_SWAP_EXPECTS_EXACTLY_9_NOTE_INPUTS: u32 = 131079; // 131079 0x00020007 | ||
| const ERR_SWAP_REQUIRES_EXACTLY_1_NOTE_ASSET: u32 = 131080; // 131080 0x00020008 |
There was a problem hiding this comment.
These are technically not part of the kernel. I think we should break them up into a different category (and maybe use 0x0003 prefix for them).
miden-tx/src/error.rs
Outdated
| const ERR_NOTE_INVALID_TAG_PREFIX_FOR_TYPE: u32 = 131141; | ||
| const ERR_NOTE_TAG_MUST_BE_U32: u32 = 131142; | ||
| // --- ACCOUNT ERRORS ----------------------------------------------------------------------------- | ||
| const ERR_ACCOUNT_CODE_MUST_BE_UPDATABLE: u32 = 131133; // 131133 0x0002003D 131072 0x00020000 |
There was a problem hiding this comment.
Why are there two separate codes here?
There was a problem hiding this comment.
I'm preparing to change codes to be in in ascending order (as soon as constants get their final names and hence correct alphabetical order). And not to mess everything up and confuse the old and new values when I will update the actual constants in masm code I wrote the old values for every constant and a starting point for a new values.
miden-tx/src/error.rs
Outdated
| // --- ACCOUNT ERRORS ----------------------------------------------------------------------------- | ||
| const ERR_ACCOUNT_CODE_MUST_BE_UPDATABLE: u32 = 131133; // 131133 0x0002003D 131072 0x00020000 | ||
| const ERR_ACCOUNT_ID_INVALID_INSUFFICIENT_NUMBER_OF_ONES: u32 = 131132; // 131132 0x0002003C | ||
| const ERR_ACCOUNT_MUST_BE_A_FAUCET_TO_CALL_PROCEDURE: u32 = 131073; // 131073 0x00020001 |
There was a problem hiding this comment.
When does this error happen?
There was a problem hiding this comment.
We use it here in the api.masm to make sure that this get_fungible_faucet_total_issuance procedure is called by a fungible faucet. But now name of the constant looks a bit odd for me. Perhaps something like ERR_ACCOUNT_MUST_BE_A_FUNGIBLE_FAUCET would be better.
miden-tx/src/error.rs
Outdated
| const ERR_ACCOUNT_SEED_DIGEST_MISMATCH: u32 = 131134; // 131134 0x0002003E | ||
|
|
||
| // --- NONCE ERROR -------------------------------------------------------------------------------- | ||
| const ERR_NONCE_DID_NOT_INCREASE_AFTER_STATE_CHANGED: u32 = 131081; // 131081 0x00020009 |
There was a problem hiding this comment.
This is probably should go into the "account errors" category as well.
Also, do we have the opposite error: "account nonce increased without state change"?
There was a problem hiding this comment.
Also, do we have the opposite error: "account nonce increased without state change"?
For now we don't have this error and as far as I can see we don't check for this anywhere. Should I create a constant for this error?
miden-tx/src/error.rs
Outdated
|
|
||
| // --- VAULT ERRORS ------------------------------------------------------------------------------- | ||
| const ERR_VAULT_ADDING_FUNGIBLE_ASSET_WOULD_EXCEED_MAX_AMOUNT_OF_9223372036854775807: u32 = 131117; // 131117 0x0002002D | ||
| const ERR_VAULT_ADD_FUNGIBLE_ASSET_TO_ACCOUNT_FAILED_INITIAL_VALUE_INVALID: u32 = 131118; // 131118 0x0002002E |
There was a problem hiding this comment.
When odes this error happen?
There was a problem hiding this comment.
We use it here in the asset_vault.masm to make sure that old vault value is correct.
miden-tx/src/error.rs
Outdated
| const ERR_ASSET_EXCEED_MAX_AMOUNT_OF_9223372036854775807: u32 = 131138; // 131138 0x00020042 | ||
|
|
||
| // --- FUNGIBLE ASSET ERRORS ---------------------------------------------------------------------- | ||
| const ERR_FUNGIBLE_ASSET_DISTRIBUTE_WOULD_CAUSE_MAX_SUPPLY_TO_BE_EXCEEDED: u32 = 131105; // 131105 0x00020021 |
There was a problem hiding this comment.
I think this also might not really be a part of the kernel.
There was a problem hiding this comment.
I'm not sure where to place this error (and also the ERR_ASSET_EXCEED_MAX_AMOUNT_OF_9223372036854775807 above it). They could be described as "faucet" and "asset" errors respectively, but creating a new section with a new prefix just for one error looks like an overkill for me.
Fumuran
force-pushed
the
andrew-rework-error-naming
branch
from
7593fa0 to
3c3df98
Compare
|
@Overcastan could you rebase this from the latest |
Fumuran
force-pushed
the
andrew-rework-error-naming
branch
from
3c3df98 to
63682a7
Compare
|
My initial thought was to update the error values to make them go in ascending order, but now I think that it doesn't make a lot of sense: probably new errors will be added to the groups, so this order will be broken anyway. |
Fumuran
force-pushed
the
andrew-rework-error-naming
branch
from
2c79cc5 to
b76f68b
Compare
|
This rebase was large and nasty, so commit history now looks odd, but at least this PR now have only right changes. |
|
Superseded by #906. |
This PR updates kernel assertion error names and descriptions so that the essence of the error is clear only by name.
Related issue: #671