Improved version of Deposit contract #51
Conversation
| Vec::new() | ||
| }; | ||
|
|
||
| bytesrepr::deserialize(arg_bytes).map_err(|_| invalid) |
There was a problem hiding this comment.
We loose the actual error cause here. it is a deserialization error but the message is lost and potentially mapped to a wrong invalid type. The user of this does not know that you are going to do that and I could pass in invalid to be ConnectionError and now anyone debugging this would never know what kind of error this was
There was a problem hiding this comment.
This will propagate a Casper APIError to the contract runtime. I did not write any of the code in detail.rs and it was used by Casper Labs in the cep standards.
There was a problem hiding this comment.
It MIGHT be propagated to ApiError, but it does not have to - depending of what called will do with error. So far I am not worried about losing error details, but rather about this error not being used in the code at all - is arguments handling done properly?
There was a problem hiding this comment.
What do you mean by "is argument handling done properly"?
There was a problem hiding this comment.
@jonas089 Please don't resolve every conversation once you posted a comment, especially one that involves further discussion. About error handling issues, see related comment.
There was a problem hiding this comment.
Error handling has been improved in the latest verison.
jonas089
force-pushed
the
feature/deposit-contract
branch
from
4b4317f to
59d2d41
Compare
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
Co-authored-by: Marijan Petričević <[email protected]>
…e, explain security and change function names
marijanp
force-pushed
the
feature/deposit-contract
branch
from
947e278 to
40822ed
Compare
Deposit contract suggestions
|
|
||
| - name: kairos-contracts x86_64-darwin | ||
| if: matrix.os == 'macos-latest' | ||
| run: nix build -L --no-link --show-trace .#packages.x86_64-darwin.kairos-contracts |
There was a problem hiding this comment.
Let's not add back x86_64-darwin.
aarch64-darwin is enough
| - name: kairos-contracts x86_64-darwin | |
| if: matrix.os == 'macos-latest' | |
| run: nix build -L --no-link --show-trace .#packages.x86_64-darwin.kairos-contracts |
There was a problem hiding this comment.
Will be addressed in other PR as discussed.
Bincode was unnecessarily included with default features, so it included panic handlers from `std`. It is better to make contract a `no_std`, so it's smaller. For such case, panic handlers can be registered with `no-std-helpers` feature in `casper-contract`.
For now we can stick to unstable one, to avoid heavy CI/CD.
Final demo contract suggestions.
| @@ -1,7 +1,6 @@ | |||
| name: check | |||
| on: | |||
| pull_request: | |||
| branches: [main] | |||
This deposit smart contract targets the release branch of the official casper node repository, e.g. casper-network/release-1.5.6.
Contract types were introduced to write a serialized Deposit to the contract's event storage.