feat: deprecating schedules array in favour of single schedule #4
Conversation
program/src/state.rs
Outdated
|
|
||
| pub fn unpack_schedules(input: &[u8]) -> Result<Vec<VestingSchedule>, ProgramError> { | ||
| let number_of_schedules = input.len() / VestingSchedule::LEN; | ||
| let mut output: Vec<VestingSchedule> = Vec::with_capacity(number_of_schedules); | ||
| let mut offset = 0; | ||
| for _ in 0..number_of_schedules { | ||
| output.push(VestingSchedule::unpack_from_slice( | ||
| &input[offset..offset + VestingSchedule::LEN], | ||
| )?); | ||
| offset += VestingSchedule::LEN; | ||
| } | ||
| pub fn unpack_schedule(input: &[u8]) -> Result<VestingSchedule, ProgramError> { | ||
| let offset = 0; | ||
| let output: VestingSchedule = VestingSchedule::unpack_from_slice( | ||
| &input[offset..offset + VestingSchedule::LEN], | ||
| )?; | ||
| Ok(output) | ||
| } |
There was a problem hiding this comment.
offset seems unnecessary now, can you do something like this?
pub fn unpack_schedule(input: &[u8]) -> Result<VestingSchedule, ProgramError> {
VestingSchedule::unpack_from_slice(&input[..VestingSchedule::LEN])
}the validation of the len happens inside the unpack_from_slice function, i cant' comment on it now, but it does src < 16 and 16 is the LEN, I recommend using the LEN const there to avoid magic numbers. same happens in this function but when used with the Header
program/src/state.rs
Outdated
| pub fn pack_schedule_into_slice(schedule: VestingSchedule, target: &mut [u8]) { | ||
| let offset = 0; | ||
| schedule.pack_into_slice(&mut target[offset..]); | ||
| } |
There was a problem hiding this comment.
this one is similar to the other one, some validation perhaps can be added so the packing doesn't fail (although the validation may happen in the pack_into_slice fn), and the offset is 0 so it can be removed and simplified. target is already a mutable reference, so not sure if you need to cast it without the offset, but not 100% certain as the mutable reference will transform the original variable.
pub fn pack_schedule_into_slice(schedule: VestingSchedule, target: &mut [u8]) {
schedule.pack_into_slice(target);
}either way, proper validation is the main thing i would check for. for example, it seems the pack_into_slice function requires 16 bytes, but we have no guarantee that target fits the required length.
edit: this seems to be somewhat checked in another file which seems to be the main function, either way, i will leave the comment just in case
program/src/processor.rs
Outdated
| release_time: schedule.release_time, | ||
| amount: schedule.amount, | ||
| }; | ||
| state_schedule.pack_into_slice(&mut data[offset..]); |
There was a problem hiding this comment.
do you think it would be more explicit to remove offset variable and just use VettingScheduleHeader::LEN or rename offset to header_offset so it's clear this starts packing what's after the header?
The original codebase https://github.com/Bonfida/token-vesting supports an array of Dates to define a Vesting Contract. In that way, it can say: "unlock 10 at May 10, 10 at May 20, ..." and so on.
The intended behaviour is to have only 1 Date per vesting / lockup, this PR replaces the Schedules<> array for a single Schedule, and deprecates the variable
number_of_schedules