-
Notifications
You must be signed in to change notification settings - Fork 925
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Improve documentation of error codes #418
Conversation
Pull Request Test Coverage Report for Build 2703406391
💛 - Coveralls |
6b8fd28
to
220d478
Compare
docs/error_codes.md
Outdated
- `GS026`: `Invalid owner provided` | ||
- **GS020: Signatures data too short** | ||
There are less signatures than the owners threshold. | ||
Provide as many as signatures as the owners threshold. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is less about the signatures provide, more about the length of the bytes data. For this it probably makes sense to further explain how the signature data is created.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe instead add an example, add a doc link that contains examples:
The length of required signatures is less than 65*threeshold.
Each signature has a constant length of 65 bytes `{32-bytes r}{32-bytes s}{1-byte v}`. If more data is necessary it can be appended to the end.
More information about signatures:
https://docs.gnosis-safe.io/contracts/signatures
What do you think?
docs/error_codes.md
Outdated
Wrong contract `v=0` signature because `startingPosition + contractSignatureLen` is out of bounds. | ||
|
||
- **GS024: Invalid contract signature provided** | ||
The EIP1271 signature provided is wrong. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
WE should link the EIP here. Also please use EIP-1271
(which the -
) ;)
- **GS024: Invalid contract signature provided** | ||
The EIP1271 signature provided is wrong. | ||
If you don't want to use this type of signature review the `v` value (0 for a contract signature). | ||
The `hash` to generate the `signature` must be calculated by the account address provided in `r` value. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how helpful this comment is without further context.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I included that because I had some issues with this when I was working on tests for EIP-1271. To me in the past would be helpful :)
The owner provided on 'r' has not pre-approved the safeTxHash. | ||
To pre-approve the safeTxHash call `approveHash` with the safeTxHash calculated by the owner. | ||
|
||
- **GS026: Invalid owner provided** |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We more info here. This error can happen also if the nonce has changed and therefore the Safe transaction hash is different than expected.
docs/error_codes.md
Outdated
Review that a correct owner is being used. | ||
|
||
- **GS031: Method can only be called from this contract** | ||
This method is only meant to be called from the contract itself. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would leave out descriptions for error messages that are self explanatory
Closes #386