feat(core): implement getBlockCommitment RPC endpoint
#150
Conversation
crates/core/src/rpc/accounts_data.rs
Outdated
|
|
||
| Ok(RpcBlockCommitment { | ||
| commitment: Some(commitment_array), | ||
| total_stake: 150, |
There was a problem hiding this comment.
Is there any significance to the 150 number we've chosen here? If we're going to hardcode something, I'd prefer 0 so it's more clearly indicating there are no commitments/stakes.
There was a problem hiding this comment.
no significance really only that we are mocking to show that confirmed blocks should have some stake
There was a problem hiding this comment.
I think let's zero out, since for surfpool there isn't really a concept of stake
crates/core/src/rpc/accounts_data.rs
Outdated
| } | ||
|
|
||
| Ok(RpcBlockCommitment { | ||
| commitment: Some(commitment_array), |
There was a problem hiding this comment.
We are always returning Some(...), but if I query an old block on the RPC, I'll see a null result.
There was a problem hiding this comment.
@MicaiahReid there is blockCommitmentCache on the RPC client that holds data about the block commitment (surfpool doesn't have this cache or need it)...the thing about the cache is that old finalized blocks with lesser chance of being rollback leave the cache...returning null when you query their commitment array. Can implement null if you want but this was the idea
There was a problem hiding this comment.
Gotcha, then I'm okay with always returning Some([0u64; 32]) for valid requests
crates/core/src/rpc/accounts_data.rs
Outdated
| @@ -637,4 +666,106 @@ mod tests { | |||
| } | |||
| ); | |||
| } | |||
|
|
|||
| #[test] | |||
| fn test_get_block_commitment_basic() { | |||
There was a problem hiding this comment.
This test isn't asserting anything - can we remove if the other tests are covering its cases?
There was a problem hiding this comment.
yeah that was just to show the return value will remove it after review and linting checks
There was a problem hiding this comment.
Let's get these sorts of things and lints fixed before review to save the back and forth! 👍
getBlockCommitment RPC endpoint
No description provided.