Skip to content
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

Timestamp to block number #183

Merged
merged 13 commits into from
Jun 15, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion contracts/starknet/Interfaces/IVotingStrategy.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ from starkware.cairo.common.uint256 import Uint256
@contract_interface
namespace IVotingStrategy:
func get_voting_power(
block : felt,
timestamp : felt,
voter_address : Address,
params_len : felt,
params : felt*,
Expand Down
42 changes: 20 additions & 22 deletions contracts/starknet/Space.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -428,7 +428,7 @@ func get_cumulative_voting_power{syscall_ptr : felt*, pedersen_ptr : HashBuiltin

let (user_voting_power) = IVotingStrategy.get_voting_power(
contract_address=voting_strategy,
block=current_timestamp,
timestamp=current_timestamp,
voter_address=voter_address,
params_len=voting_strategy_params_len,
params=voting_strategy_params,
Expand Down Expand Up @@ -680,8 +680,11 @@ func vote{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : fe
end

let (proposal) = proposal_registry_store.read(proposal_id)
let (current_timestamp) = get_block_timestamp()

# The snapshot timestamp at which voting power will be taken
let snapshot_timestamp = proposal.snapshot_timestamp

let (current_timestamp) = get_block_timestamp()
# Make sure proposal is still open for voting
with_attr error_message("Voting period has ended"):
assert_lt(current_timestamp, proposal.max_end_timestamp)
Expand Down Expand Up @@ -713,7 +716,7 @@ func vote{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : fe
)

let (user_voting_power) = get_cumulative_voting_power(
current_timestamp,
snapshot_timestamp,
voter_address,
used_voting_strategies_len,
used_voting_strategies,
Expand Down Expand Up @@ -748,7 +751,6 @@ func propose{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr :
execution_hash : Uint256,
metadata_uri_len : felt,
metadata_uri : felt*,
ethereum_block_number : felt,
executor : felt,
used_voting_strategies_len : felt,
used_voting_strategies : felt*,
Expand All @@ -759,26 +761,23 @@ func propose{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr :
) -> ():
alloc_locals

# We cannot have `0` as the `ethereum_block_number` because we rely on checking
# if it's different than 0 in `finalize_proposal`.
with_attr error_message("Invalid block number"):
assert_not_zero(ethereum_block_number)
end

# Verify that the caller is the authenticator contract.
assert_valid_authenticator()

# Verify that the executor address is one of the whitelisted addresses
assert_valid_executor(executor)

let (current_timestamp) = get_block_timestamp()
# The snapshot for the proposal is the current timestamp at proposal creation
# We use a timestamp instead of a block number to define a snapshot so that the system can generalize to multi-chain
# TODO: Need to consider what sort of guarantees we have on the timestamp returned being correct.
let (snapshot_timestamp) = get_block_timestamp()
Orland0x marked this conversation as resolved.
Show resolved Hide resolved
let (delay) = voting_delay_store.read()

let (_min_voting_duration) = min_voting_duration_store.read()
let (_max_voting_duration) = max_voting_duration_store.read()

# Define start_timestamp, min_end and max_end
let start_timestamp = current_timestamp + delay
let start_timestamp = snapshot_timestamp + delay
let min_end_timestamp = start_timestamp + _min_voting_duration
let max_end_timestamp = start_timestamp + _max_voting_duration

Expand All @@ -788,7 +787,7 @@ func propose{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr :
)

let (voting_power) = get_cumulative_voting_power(
start_timestamp,
snapshot_timestamp,
proposer_address,
used_voting_strategies_len,
used_voting_strategies,
Expand All @@ -815,10 +814,10 @@ func propose{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr :
let proposal = Proposal(
execution_hash,
_quorum,
snapshot_timestamp,
start_timestamp,
min_end_timestamp,
max_end_timestamp,
ethereum_block_number,
hash,
executor,
)
Expand Down Expand Up @@ -862,16 +861,15 @@ func finalize_proposal{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_c
let (proposal) = proposal_registry_store.read(proposal_id)
with_attr error_message("Invalid proposal id"):
# Checks that the proposal id exists. If it doesn't exist, then the whole `Proposal` struct will
# be set to 0, hence `ethereum_block_number` will be set to 0 too.
assert_not_zero(proposal.ethereum_block_number)
# be set to 0, hence the snapshot timestamp will be set to 0 too.
assert_not_zero(proposal.snapshot_timestamp)
end

# Make sure proposal period has ended
# NOTE: commented out the `with_attr` block because it needs 0.8.1 to work
# with_attr error_message("Min voting period has not elapsed"):
let (current_timestamp) = get_block_timestamp()
assert_le(proposal.min_end_timestamp, current_timestamp)
# end
with_attr error_message("Min voting period has not elapsed"):
assert_le(proposal.min_end_timestamp, current_timestamp)
end

# Make sure execution params match the stored hash
let (recovered_hash) = hash_array(execution_params_len, execution_params)
Expand Down Expand Up @@ -958,8 +956,8 @@ func cancel_proposal{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_che
let (proposal) = proposal_registry_store.read(proposal_id)
with_attr error_message("Invalid proposal id"):
# Checks that the proposal id exists. If it doesn't exist, then the whole `Proposal` struct will
# be set to 0, hence `ethereum_block_number` will be set to 0 too.
assert_not_zero(proposal.ethereum_block_number)
# be set to 0, hence the snapshot timestamp will be set to 0 too.
assert_not_zero(proposal.snapshot_timestamp)
end

let proposal_outcome = ProposalOutcome.CANCELLED
Expand Down
22 changes: 16 additions & 6 deletions contracts/starknet/VotingStrategies/SingleSlotProof.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ from contracts.starknet.fossil.contracts.starknet.types import StorageSlot
from contracts.starknet.lib.general_address import Address
from contracts.starknet.lib.slot_key import get_slot_key
from contracts.starknet.lib.words import words_to_uint256
from contracts.starknet.lib.timestamp import get_eth_block_number, l1_headers_store

# FactRegistry simplified interface
@contract_interface
Expand All @@ -33,17 +34,18 @@ end

@constructor
func constructor{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}(
fact_registry : felt
fact_registry_address : felt, l1_headers_store_address : felt
):
fact_registry_store.write(value=fact_registry)
fact_registry_store.write(value=fact_registry_address)
l1_headers_store.write(value=l1_headers_store_address)
return ()
end

@view
func get_voting_power{
syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr, bitwise_ptr : BitwiseBuiltin*
}(
block : felt,
timestamp : felt,
voter_address : Address,
params_len : felt,
params : felt*,
Expand All @@ -52,7 +54,10 @@ func get_voting_power{
) -> (voting_power : Uint256):
alloc_locals

let (local fact_registry_addr) = fact_registry_store.read()
let (fact_registry_addr) = fact_registry_store.read()

let (eth_block_number) = get_eth_block_number(timestamp)
let eth_block_number = eth_block_number - 1 # temp shift - waiting for Fossil fix
Orland0x marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor Author

Choose a reason for hiding this comment

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


# Decoding voting strategy parameters
let (
Expand All @@ -70,7 +75,11 @@ func get_voting_power{
# contract address where the desired slot resides, and the section element is the index of the slot in that contract.
assert params_len = 2
let contract_address = params[0]
let slot_index = params[1]

# In the current implementation, we cant store arrays with a zero as the last element because the read function would just treat
# the array is 1 element shorter. Hence, we offset the slot_index by 1 so that it is never zero.
# Probably worth changing our array handling so this is not necessary.
let slot_index = params[1] - 1
Copy link
Contributor Author

@Orland0x Orland0x Jun 15, 2022

Choose a reason for hiding this comment

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

this isnt ideal. we could change how we store arrays.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah didn't comment on that since I saw this comment. Def agree 🤝

Copy link
Contributor Author

Choose a reason for hiding this comment

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

opened an issue: #188


# Checking slot proof is for the correct slot
let (valid_slot) = get_slot_key(slot_index, voter_address.value)
Expand All @@ -83,7 +92,7 @@ func get_voting_power{

let (voting_power) = IFactsRegistry.get_storage_uint(
fact_registry_addr,
block,
eth_block_number,
contract_address,
slot,
proof_sizes_bytes_len,
Expand All @@ -93,6 +102,7 @@ func get_voting_power{
proofs_concat_len,
proofs_concat,
)

# If any part of the voting strategy calculation is invalid, the voting power returned should be zero
return (voting_power)
end
Expand Down
2 changes: 1 addition & 1 deletion contracts/starknet/VotingStrategies/Vanilla.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ from contracts.starknet.lib.general_address import Address
# Returns a voting power of 1 for every address it is queried with.
@view
func get_voting_power{range_check_ptr}(
block : felt,
timestamp : felt,
voter_address : Address,
params_len : felt,
params : felt*,
Expand Down
2 changes: 1 addition & 1 deletion contracts/starknet/VotingStrategies/Whitelist.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ end

@view
func get_voting_power{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr : felt}(
block : felt,
timestamp : felt,
voter_address : Address,
params_len : felt,
params : felt*,
Expand Down
2 changes: 1 addition & 1 deletion contracts/starknet/fossil
2 changes: 1 addition & 1 deletion contracts/starknet/lib/proposal.cairo
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@ from starkware.cairo.common.uint256 import Uint256
struct Proposal:
member execution_hash : Uint256
member quorum : Uint256
member snapshot_timestamp : felt
member start_timestamp : felt
member min_end_timestamp : felt
member max_end_timestamp : felt
member ethereum_block_number : felt
member execution_params_hash : felt
member executor : felt
end
37 changes: 37 additions & 0 deletions contracts/starknet/lib/timestamp.cairo
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
%lang starknet

from starkware.cairo.common.cairo_builtins import HashBuiltin

@contract_interface
namespace IL1HeadersStore:
func get_latest_l1_block() -> (number : felt):
end
end

@storage_var
func l1_headers_store() -> (res : felt):
end

@storage_var
func timestamp_to_eth_block_number(timestamp : felt) -> (number : felt):
end

func get_eth_block_number{syscall_ptr : felt*, pedersen_ptr : HashBuiltin*, range_check_ptr}(
timestamp : felt
) -> (number : felt):
let (number) = timestamp_to_eth_block_number.read(timestamp)
if number != 0:
# The timestamp has already be queried in fossil and stored. Therefore we can just return the stored value
# This branch will be taken whenever a vote is cast as the mapping value would be set at proposal creation.
return (number)
else:
# The timestamp has not yet been queried in fossil. Therefore we must query Fossil for the latest eth block
Copy link
Contributor

Choose a reason for hiding this comment

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

This means a proposer can't create a proposal with a snapshot that was BEFORE the creation of the proposal right? Like if I create a proposal on second of Jan and I want the snapshot to be taken on the 1st of Jan (one day before), that's not possible, correct?

Copy link
Contributor Author

@Orland0x Orland0x Jun 15, 2022

Choose a reason for hiding this comment

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

Well technically you could if the latest block number stored in Fossil was on the 1st. This shouldnt happen though as I expect block hashes to be sent hourly.

The ethereum block number snapshot will always be sometime before the proposal creation, the length of which depends on the regularity of block hashes being sent from L1

# number stored there and store it here in the mapping indexed by the timestamp provided.
# This branch will be taken whenever a proposal is created, except for the (rare) case of multiple proposals
Orland0x marked this conversation as resolved.
Show resolved Hide resolved
# being created in the same block.
let (l1_headers_store_address) = l1_headers_store.read()
let (number) = IL1HeadersStore.get_latest_l1_block(l1_headers_store_address)
timestamp_to_eth_block_number.write(timestamp, number)
return (number)
end
end
13 changes: 1 addition & 12 deletions scripts/deployTestSpace.ts
Original file line number Diff line number Diff line change
@@ -1,16 +1,5 @@
import fs from 'fs';
import {
Contract,
Account,
defaultProvider,
ec,
encode,
hash,
json,
number,
stark,
RawCalldata,
} from 'starknet';
import { defaultProvider, json } from 'starknet';
import { SplitUint256 } from '../test/shared/types';
import { flatten2DArray } from '../test/shared/helpers';

Expand Down
23 changes: 10 additions & 13 deletions test/crosschain/EthTxAuth.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { StarknetContract, Account, HttpNetworkConfig } from 'hardhat/types';
import { strToShortStringArr } from '@snapshot-labs/sx';
import { createExecutionHash, getCommit, getProposeCalldata } from '../shared/helpers';
import { ethTxAuthSetup } from '../shared/setup';
import { PROPOSE_SELECTOR, VOTE_SELECTOR } from '../shared/constants';
import { PROPOSE_SELECTOR } from '../shared/constants';

// Dummy tx
const tx1 = {
Expand Down Expand Up @@ -49,7 +49,6 @@ describe('L1 interaction with Snapshot X', function () {
let userVotingParamsAll: bigint[][];
let executionStrategy: bigint;
let executionParams: bigint[];
let ethBlockNumber: bigint;
let proposeCalldata: bigint[];

before(async function () {
Expand All @@ -71,7 +70,6 @@ describe('L1 interaction with Snapshot X', function () {
'Hello and welcome to Snapshot X. This is the future of governance.'
);
proposerEthAddress = signer.address;
ethBlockNumber = BigInt(1337);
spaceAddress = BigInt(space.address);
usedVotingStrategies = [BigInt(vanillaVotingStrategy.address)];
userVotingParamsAll = [[]];
Expand All @@ -81,7 +79,6 @@ describe('L1 interaction with Snapshot X', function () {
proposerEthAddress,
executionHash,
metadataUri,
ethBlockNumber,
executionStrategy,
usedVotingStrategies,
userVotingParamsAll,
Expand All @@ -94,12 +91,12 @@ describe('L1 interaction with Snapshot X', function () {
// Committing the hash of the payload to the StarkNet Commit L1 contract
await starknetCommit
.connect(signer)
.commit(getCommit(BigInt(space.address), PROPOSE_SELECTOR, proposeCalldata));
.commit(getCommit(spaceAddress, PROPOSE_SELECTOR, proposeCalldata));
// Checking that the L1 -> L2 message has been propogated
expect((await starknet.devnet.flush()).consumed_messages.from_l1).to.have.a.lengthOf(1);
// Creating proposal
await ethTxAuthenticator.invoke('authenticate', {
target: BigInt(space.address),
target: spaceAddress,
function_selector: PROPOSE_SELECTOR,
calldata: proposeCalldata,
});
Expand All @@ -109,17 +106,17 @@ describe('L1 interaction with Snapshot X', function () {
await starknet.devnet.loadL1MessagingContract(networkUrl, mockStarknetMessaging.address);
await starknetCommit
.connect(signer)
.commit(getCommit(BigInt(space.address), PROPOSE_SELECTOR, proposeCalldata));
.commit(getCommit(spaceAddress, PROPOSE_SELECTOR, proposeCalldata));
await starknet.devnet.flush();
await ethTxAuthenticator.invoke('authenticate', {
target: BigInt(space.address),
target: spaceAddress,
function_selector: PROPOSE_SELECTOR,
calldata: proposeCalldata,
});
// Second attempt at calling authenticate should fail
try {
await ethTxAuthenticator.invoke('authenticate', {
target: BigInt(space.address),
target: spaceAddress,
function_selector: PROPOSE_SELECTOR,
calldata: proposeCalldata,
});
Expand All @@ -132,11 +129,11 @@ describe('L1 interaction with Snapshot X', function () {
await starknet.devnet.loadL1MessagingContract(networkUrl, mockStarknetMessaging.address);
await starknetCommit
.connect(signer)
.commit(getCommit(BigInt(space.address), PROPOSE_SELECTOR, proposeCalldata)); // Wrong selector
.commit(getCommit(spaceAddress, PROPOSE_SELECTOR, proposeCalldata)); // Wrong selector
await starknet.devnet.flush();
try {
await ethTxAuthenticator.invoke('authenticate', {
target: BigInt(space.address),
target: spaceAddress,
function_selector: PROPOSE_SELECTOR,
calldata: proposeCalldata,
});
Expand All @@ -150,11 +147,11 @@ describe('L1 interaction with Snapshot X', function () {
proposeCalldata[0] = BigInt(ethers.Wallet.createRandom().address); // Random l1 address in the calldata
await starknetCommit
.connect(signer)
.commit(getCommit(BigInt(space.address), PROPOSE_SELECTOR, proposeCalldata));
.commit(getCommit(spaceAddress, PROPOSE_SELECTOR, proposeCalldata));
await starknet.devnet.flush();
try {
await ethTxAuthenticator.invoke('authenticate', {
target: BigInt(space.address),
target: spaceAddress,
function_selector: PROPOSE_SELECTOR,
calldata: proposeCalldata,
});
Expand Down
Loading