-
Notifications
You must be signed in to change notification settings - Fork 11
Owner + upgrade functionality for EVM contract #63
Conversation
I followed the dependencies to see which each of them are using and the last being |
#65 addresses this current build issue you have. Review, pull and rebase. |
Current issue is a plethora of, needs further investigating whats going on over there as we are getting this straight from git itself which indicates to me that
|
Pinned |
sdk::panic_utf8(b"LackBalanceForStorage"); | ||
} | ||
} else { | ||
// TODO: return funds to caller? |
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.
Needs discussion and completing.
if sdk::attached_deposit() | ||
< ((final_storage - initial_storage) as u128) * sdk::storage_byte_cost() | ||
{ | ||
// TODO: add how much storage needed to error? |
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.
Needs discussion and completing.
} | ||
|
||
pub fn get_owner() -> Vec<u8> { | ||
sdk::read_storage(OWNER_KEY).unwrap_or_else(Vec::new) |
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.
Should this be erroring?
} | ||
|
||
pub fn get_bridge_prover() -> Vec<u8> { | ||
sdk::read_storage(BRIDGE_PROVER_KEY).unwrap_or_else(Vec::new) |
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.
Should this be erroring?
bcd6105
to
f8fbd2e
Compare
@joshuajbouw Please sort out the merge conflict due to #65, and give me an idea about whether I should proceed to merge this as-is or you figure the sim tests will be forthcoming in short order if you're unblocked now? |
Tests are going great and working as expected so far. I can't test the actual upgrade but everything else is tested up til this point. I am currently blocked with not being able to produce blocks which will trigger the upgrade as I need to produce a days worth of blocks: near/near-sdk-rs#320 Prior, it was based off of 24 hour waiting period but, of course, we can't -exactly- wait 24 hours for us to check if the test went through and since our block times are 1 block per second, I can instead create that many blocks in the test to trigger. The tricky thing is that is a lot of blocks to produce. What I would suggest is to create some test methods in the WASM itself to produce a WASM test binary which allow us to alter values internally to simulate that a day has passed. Thoughts? |
15443ed
to
77c2414
Compare
There is a method in standalone runtime to produce a bunch of empty blocks:
If producing 86k blocks takes too long in test (I would not expect that but who knows) - let's just add the wait time as a parameter in constructor and store in storage. This way it can be configured to longer as code matures as well. Further upgrades can change that parameter on migration. |
Right, I had to add a PR to be able to produce the blocks. I do like the idea of adding in that extra argument though for time to wait. Though, it makes me wonder what the minimum value could be. To be safe, you obviously would want at least 1 day. |
Superseded by aurora-is-near/aurora-engine#6 |
Allows to set owner at
new
call and then change it.Allows to stage new code and upgrade after 24 hours.
TODO: adding sim tests