-
Notifications
You must be signed in to change notification settings - Fork 224
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
Burn functionality #4573
Burn functionality #4573
Conversation
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper <[email protected]>
Co-authored-by: igor-casper
Co-authored-by: igor-casper
Co-Authored-By: Jan Hoffmann <[email protected]>
Co-authored-by: igor-casper
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.
optimistic approval assuming asked for changes are made.
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.
Looking good, few tweaks
let data = | ||
unsafe { Vec::from_raw_parts(data_non_null_ptr.as_ptr(), arg_size, arg_size) }; | ||
if ret != 0 { | ||
return None; | ||
} | ||
data |
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 found an existing method that does the same thing in other test contracts, perhaps you can just copy and paste it from there:
casper-node/smart_contracts/contracts/client/non-standard-payment/src/main.rs
Lines 49 to 88 in 79f4f34
fn get_named_arg_if_exists<T: FromBytes>(name: &str) -> Option<T> { | |
let arg_size = { | |
let mut arg_size: usize = 0; | |
let ret = unsafe { | |
ext_ffi::casper_get_named_arg_size( | |
name.as_bytes().as_ptr(), | |
name.len(), | |
&mut arg_size as *mut usize, | |
) | |
}; | |
match api_error::result_from(ret) { | |
Ok(_) => Some(arg_size), | |
Err(ApiError::MissingArgument) => None, | |
Err(e) => runtime::revert(e), | |
} | |
}?; | |
let arg_bytes = if arg_size > 0 { | |
let res = { | |
let data_non_null_ptr = contract_api::alloc_bytes(arg_size); | |
let ret = unsafe { | |
ext_ffi::casper_get_named_arg( | |
name.as_bytes().as_ptr(), | |
name.len(), | |
data_non_null_ptr.as_ptr(), | |
arg_size, | |
) | |
}; | |
let data = | |
unsafe { Vec::from_raw_parts(data_non_null_ptr.as_ptr(), arg_size, arg_size) }; | |
api_error::result_from(ret).map(|_| data) | |
}; | |
// Assumed to be safe as `get_named_arg_size` checks the argument already | |
res.unwrap_or_revert() | |
} else { | |
// Avoids allocation with 0 bytes and a call to get_named_arg | |
Vec::new() | |
}; | |
let value = bytesrepr::deserialize(arg_bytes).unwrap_or_revert_with(ApiError::InvalidArgument); | |
Some(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.
yes...that's the one i was looking for a couple of weeks ago...couldn't remember which contract it was in
we should just add this to contract api...it's come up various times.
Co-authored-by: igor-casper <[email protected]>
execution_engine_testing/tests/src/test/system_contracts/mint.rs
Outdated
Show resolved
Hide resolved
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.
LGTM
The networking related changes LGTM, too. |
Implements casper-network/ceps#92