Skip to content

Conversation

@guibescos
Copy link
Contributor

@guibescos guibescos commented Aug 12, 2022

upd_price in rust.
Also moved 3 unit tests from C to Rust.

The boundary between C and Rust becomes the upd_aggregate function

#[error("InvalidWritableAccount")]
InvalidWritableAccount = 607,
#[error("InvalidWritableAccount")]
#[error("InvalidFreshAccount")]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This message was a typo

@guibescos guibescos changed the title Works Upd price in rust Aug 15, 2022
# Compile C code to system architecture for use by rust's cargo test
$(CC) -c ./src/oracle/for_cargo_test/cpyth_test.c -o ./target/cpyth_test.o
gcc -c ./src/oracle/for_cargo_test/cpyth_test.c -o ./target/cpyth_test.o
# Bundle C code compiled to system architecture for use by rust's cargo test
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Had to change this back... maybe $CC in the context of this makefile has the bpf flags?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm ok weird. i'm not really sure how this whole makefile process works, tbh.

Copy link
Contributor

@jayantk jayantk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome. Left you some minor comments

Also, are you going to delete the now unused C implementations?

# Compile C code to system architecture for use by rust's cargo test
$(CC) -c ./src/oracle/for_cargo_test/cpyth_test.c -o ./target/cpyth_test.o
gcc -c ./src/oracle/for_cargo_test/cpyth_test.c -o ./target/cpyth_test.o
# Bundle C code compiled to system architecture for use by rust's cargo test
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmmm ok weird. i'm not really sure how this whole makefile process works, tbh.

)
}

pub fn is_component_update(cmd_args: &cmd_upd_price_t) -> Result<bool, ProgramError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why "component"?

also, the argument to this is confusing because if you have a cmd_upd_price_t, you would expect its cmd_ field to be the corresponding int.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Component as opposed to aggregate.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is tricky : the dispatcher routes upd_price, upd_price_no_fail_on_error and agg_price to this function . The first two update the individual publisher's price, unlike agg_price which only tries to update the aggregate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah gotcha. this is fine

@guibescos guibescos marked this pull request as ready for review August 15, 2022 21:25
Copy link
Contributor

@jayantk jayantk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome!

)
}

pub fn is_component_update(cmd_args: &cmd_upd_price_t) -> Result<bool, ProgramError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah gotcha. this is fine

@guibescos guibescos merged commit d2de0ab into main Aug 16, 2022
@guibescos guibescos deleted the upd-price branch August 16, 2022 03:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants